Question

我想知道下面的代码是否可以重构/改进/简化？我在这里发布这个，因为我想要另一个程序员的视角/视图;看到别人会做什么总是很好的。

<?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; 
    } 
} 

?> 

Answer 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; 

那一脉继承和你可能会发现，很快你和你的代码的状态更快乐。