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; 

那一脈繼承和你可能會發現，很快你和你的代碼的狀態更快樂。