可以简化或改进此 PHP 代码吗？答案

【问题标题】：Could this PHP code be simplified or improved?可以简化或改进此 PHP 代码吗？
【发布时间】：2011-05-27 14:59:18
【问题描述】：

我想知道是否可以重构/改进/简化以下代码？我在这里发布这个，因为我想要另一个程序员的观点/观点；看看别人会怎么做总是好的。

<?php

function moderateTopic($topic_id, $action = NULL) {
    $locked_query       = "SELECT topic_id FROM forum_topics WHERE status = 'locked' AND topic_id = '{$topic_id}'";
    $locked_count       = totalResults($locked_query);
    $announcement_query = "SELECT topic_id FROM forum_topics WHERE topic_type = 2 AND topic_id = '{$topic_id}'";
    $announcement_count = totalResults($announcement_query);
    $sticky_query        = "SELECT topic_id FROM forum_topics WHERE topic_type = 3 AND topic_id = '{$topic_id}'";
    $sticky_count       = totalResults($sticky_query);

    if (is_null($action) == FALSE) {
        switch ($action) {
            case 1:
                if ($locked_count > 0) {
                    $topic_query = "UPDATE forum_topics SET status = 'unlocked' WHERE topic_id = '{$topic_id}'";
                } else {
                    $topic_query = "UPDATE forum_topics SET status = 'locked' WHERE topic_id = '{$topic_id}'";
                }
                doQuery($topic_query);
                break;
            case 2:
                if ($announcement_count > 0) {
                    $topic_query = "UPDATE forum_topics SET topic_type = 1 WHERE topic_id = '{$topic_id}'";
                } else {
                    $topic_query = "UPDATE forum_topics SET topic_type = 2 WHERE topic_id = '{$topic_id}'";
                }
                doQuery($topic_query);
                break;
            case 3:
                if ($sticky_count > 0) {
                    $topic_query = "UPDATE forum_topics SET topic_type = 1 WHERE topic_id = '{$topic_id}'";
                } else {
                    $topic_query = "UPDATE forum_topics SET topic_type = 3 WHERE topic_id = '{$topic_id}'";
                }
                doQuery($topic_query);
                break;
            case 4:
                header('Location: ' . urlify(9, $topic_id));
                break;
            case 5:
                header('Location: ' . urlify(11, $topic_id));
                break;
        }
        header('Location: ' . urlify(2, $topic_id));
    } else {
        $locked       = $locked_count > 0 ? 'Unlock' : 'Lock';
        $announcement = $announcement_count > 0 ? 'Unannounce' : 'Announce';
        $sticky       = $sticky_count > 0 ? 'Unsticky' : 'Sticky';

        return <<<EOT
<div style="float: right;">
<form method="POST">
<select name="action" onChange="document.forms[0].submit();">
<option value="">- - Moderate - -</option>
<option value="1"> =&gt; {$locked}</option>
<option value="2"> =&gt; {$announcement}</option>
<option value="3"> =&gt; {$sticky}</option>
<option value="4"> =&gt; Move</option>
<option value="5"> =&gt; Delete</option>
</select>
</form>
</div>
EOT;
    }
}

?>

【问题讨论】：

refactormycode.com 是这类问题的更好选择。
@therefromhere 你链接到的这个蹩脚的网站无法让任何代码变得更好。最坏，最有可能
@Col。哈哈，我承认我没用过。但是 Stackoverflow 引擎并不真正适合代码重构。也许有更好的选择。
将前 3 个查询合并为一个 "SELECT * FROM forum_topics WHERE topic_id = ".intval($topic_id)，然后减少分支案例，使其仅更改必要的变量。看到这个问题的例子：stackoverflow.com/questions/4359642/… 也不要从这个函数返回任何 HTML，而是一个数据数组并学习使用模板。就是这样
@therefromhere 实际上，不需要引擎。但仅限于大脑和经验。

标签： php refactoring

【解决方案1】：

我会从寻找重复开始。我看到 6 个非常相似的查询字符串出现：

$topic_query = "UPDATE forum_topics SET status = 'unlocked' WHERE topic_id = '{$topic_id}'";
$topic_query = "UPDATE forum_topics SET status = 'locked' WHERE topic_id = '{$topic_id}'";
$topic_query = "UPDATE forum_topics SET topic_type = 1 WHERE topic_id = '{$topic_id}'";
$topic_query = "UPDATE forum_topics SET topic_type = 2 WHERE topic_id = '{$topic_id}'";
$topic_query = "UPDATE forum_topics SET topic_type = 1 WHERE topic_id = '{$topic_id}'";
$topic_query = "UPDATE forum_topics SET topic_type = 3 WHERE topic_id = '{$topic_id}'";

每一个都跟随着一个查询的执行。那么方法呢？

function setField($topic_id, $field, $value) {
    $topic_query = "UPDATE forum_topics SET '{$field}' = '{$value}' WHERE topic_id = '{$topic_id}'";
    doQuery($topic_query);
}

现在你的 switch 语句的第一部分看起来像这样：

switch ($action) {
    case 1:
        if ($locked_count > 0) {
            setField($topic_id, 'status', 'unlocked');
        } else {
            setField($topic_id, 'status', 'locked');
        }
        break;
    case 2:
        if ($announcement_count > 0) {
            setField($topic_id, 'topic_type', 1);
        } else {
            setField($topic_id, 'topic_type', 2);
        }
        break;
    case 3:
        if ($sticky_count > 0) {
            setField($topic_id, 'topic_type', 1);
        } else {
            setField($topic_id, 'topic_type', 3);
        }
        break;

但我发现我搞砸了——有时该字段是一个字符串，有时是一个 int——而 int 案例都是 topic_type。所以让它像这样工作：

switch ($action) {
    case 1:
        if ($locked_count > 0) {
            setField($topic_id, 'status', 'unlocked');
        } else {
            setField($topic_id, 'status', 'locked');
        }
        break;
    case 2:
        if ($announcement_count > 0) {
            setTopicType($topic_id, 1);
        } else {
            setTopicType($topic_id, 2);
        }
        break;
    case 3:
        if ($sticky_count > 0) {
            setTopicType($topic_id, 1);
        } else {
            setTopicType($topic_id, 3);
        }
        break;

按照这种思路继续下去，您很快就会发现自己对代码的状态更加满意了。

【讨论】：