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']);


      }

我已将功能代码放入馅饼中。

格式更新功能http://pastie.org/1213958

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

编辑这两个函数都来自不同的文件，它们包含在 home.php 中，formatUpdate 来自 functions.php getUserUpdates 来自 queries.php

4

2 回答 2

2

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

其次，这就是 coreyward 所说的 lambda：

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

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

第三，我同意 webbiedave 关于回声语句的观点。避免这种情况的好方法：在应用程序的“核心”中，将所有 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 处理的字段，则不必担心每次调用它时都在代码中搜索以更改参数。
于 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条语句，因为此时运行的任何代码都只会在$username != $_SESSION['u_name'].

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

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

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

于 2010-10-11T21:16:57.593 回答