php 重构和更好的 php 编码？

Question

我有这个过程是我的应用程序的核心，我正在创建它，但出于某种原因，我觉得这是最糟糕的方法（本能），我想看看这个过程是否有问题，我是否以一种糟糕的方式接近它！ p.s.代码工作正常，只是重构问题。

过程是：

用户去主页，他们看到他们的最新活动，由其他网站成员（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']);


      }

我已经把函数代码放在了paste中。

格式更新函数http://pastie.org/1213958

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

EDIT 这两个函数来自不同的文件，它们包含在 home.php 中， 来自functions.php的格式更新 从 query.php 获取用户更新

Answer 1

首先，最好有单独的函数来获取数据和格式化数据。这是重构代码的良好开端。它使将来变得更容易：如果您想以不同的方式格式化数据，只需扩展格式化程序即可。

第二，这就是 coreyward 所说的 lambda：

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

删除function 关键字。在定义函数时使用function。但在这里你只打电话给一个。 （您在 queries.php 中定义了它。）

第三，我同意 webbiedave 关于 echo 语句的观点。避免这种情况的好方法：在应用程序的“核心”中，将所有 HTML 收集到一个位置。然后，当您收集了要在页面上显示的所有内容后，您可以一次全部回显。这样可以更轻松地跟踪您正在做的事情，并记住所有事情的顺序。它还可以更轻松地添加页眉和页脚，或进行更多格式设置。否则，如果您的代码周围散布着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()。这有两个好处：

1. 更易于阅读。
2. 如果你曾经 想要更改的字段 formatUpdate 处理，你不处理 不得不担心搜索 通过您的代码更改 每次调用它的参数。

Answer 2

首先，我认为你的意思是：

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

在您的 getUserUpdates() 函数中有一个冗余分支：

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

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

您不需要第二条if 语句，因为此时运行的任何代码都只会在$username != $_SESSION['u_name'] 时运行。

在我看来，通常最好不要让不同的函数直接在堆栈中回显 HTML（例如 echoVote()）。最好让函数返回数据并让原始调用者回显它。如果需要，这允许调用者执行额外的数据按摩。

除此之外，您的代码正在获取数据、循环并根据结果采取行动，这几乎是标准票价。

我认为你的本能是对自己有点太苛刻了；）还有待改进，但这肯定不是做任何事情的最糟糕的方式。