0

私は自分のアプリの心臓部であるこのプロセスを作成していますが、何らかの理由でそれを行うには最悪の方法のように感じます(本能)。このプロセスに何か問題があるかどうかを確認したかったのです。私は悪い方法でそれに近づきます！psコードは問題なく動作しますが、問題をリファクタリングするだけです。

プロセスは次のとおりです。

ユーザーがホームページにアクセスすると、最新のアクティビティが表示され、他のサイト メンバー (home.php) によって、

 //function to bring the latest activities from database
   $results=function getUserUpdates($_SESSION['user_id'];


while($row = mysql_fetch_array($results))

      {
//another function to format the activities in a social stream
          echo formatUpdate($row['user_note'],$row['dt'],$row['picture'],$row['username'],$row['id'],$row['reply_id'],$row['reply_name'],$row['votes_up'],$row['votes_down']);


      }

私はパスティに機能コードを入れました。

formatUpdate 関数http://pastie.org/1213958

getUserUpdates 関数http://pastie.org/1213962

EDIT両方の関数は、home.php に含まれている別のファイルからのものであり、functions.php からの formatUpdate は、queries.php からの getUserUpdates です。

4

2 に答える 2

2

まず第一に、データを取得するための関数とデータをフォーマットするための関数が別々にあると便利です。コードのリファクタリングに向けた良いスタートです。将来的には簡単になります。データを別の方法でフォーマットしたい場合は、フォーマッターを拡張するだけです。

第二に、これはコアリワードがラムダで意味するものです:

$results=function getUserUpdates($_SESSION['user_id'];

functionキーワードを削除します。関数を定義するときに使用functionします。しかし、ここでは 1 つしか呼び出していません。(querys.php で定義しました。)

3 番目に、echo ステートメントについて webbiedave に同意します。これを回避する良い方法: アプリの「心臓部」で、すべての HTML を 1 か所に集めます。次に、ページに表示するすべてのものを収集したら、一度にすべてエコーすることができます。これにより、何をしているかを追跡しやすくなり、すべての順序を覚えやすくなります。また、ヘッダーとフッターを追加したり、より多くの書式を設定したりすることも簡単になります. それ以外の場合、echoコードの周りにステートメントが散らばっている場合、そこにあるべきではない何かが抜け落ちやすくなります。

これが私が意味することの非常に基本的な例です：

$html = '';
$results = getUserUpdates($_SESSION['user_id'];

while($row = mysql_fetch_array($results)) {
    $fields = array(
        'user_note' => $row['user_note'],
        'dt' => $row['dt'],
        'picture' => $row['picture'],
        'username' => $row['username'],
        'id' => $row['id'],
        'reply_id' => $row['reply_id'],
        'reply_name' => $row['reply_name'],
        'votes_up' => $row['votes_up'],
        'votes_down' => $row['votes_down'],
    );
    $html .= formatUpdate($fields);
}
// This way you can do whatever you want to $html here.
echo $html;

また、$row のすべてのフィールドを配列に入れ、それを formatUpdate() に渡したことにも注意してください。これには 2 つの利点があります。

  1. 読みやすくなっています。
  2. formatUpdate が扱うフィールドを変更したい場合でも、呼び出すたびにコードを検索して引数を変更する必要はありません。
于 2010-10-11T22:09:27.013 に答える
1

まず、私はあなたが意味すると思います：

$results = getUserUpdates($_SESSION['user_id']);

getUserUpdates()関数には冗長なブランチがあります:

if ($username == $_SESSION['u_name']){
    // return something
}

if ($username != $_SESSION['u_name']){
    // return something else
}

ifその時点で実行されるコードは if でのみ実行されるため、2 番目のステートメントは必要ありません$username != $_SESSION['u_name']。

私の意見では、通常、スタックに直接 HTML をエコーするさまざまな関数 ( などechoVote()) を持たない方がよいでしょう。関数にデータを返させ、元の呼び出し元にそれをエコーさせることをお勧めします。これにより、呼び出し元は、必要に応じて追加のデータ マッサージを実行できます。

それ以外では、コードはデータを取得し、ループして結果に基づいて動作しますが、これはほとんど標準的な方法です。

あなたの本能は、自分自身に少し厳しすぎることだと思います;) 改善すべき点はありますが、それが何かを行うための最悪の方法ではないことは確かです.

于 2010-10-11T21:16:57.593 に答える