2013-10-18 49 views
1

我着手圍繞一堆返回生成器的類(php 5.5)做一個小項目。重構比較/運營商塊幹起來,並減少C.R.A.P水平

這個小項目的主要動機是擴展我的TDD旅程,擺弄發電機,並且有一個包可以扔在packagist上供以後使用。

整個「工程」目前的狀態可以在Github

所有的測試都是綠色的發現,這些方法我想要做什麼。現在我想重構,因爲我有很多發表。

/** 
    * Returns a Generator with a even range. 
    * 
    * getEven(10); // 10,12,14,16,18,20,22 ... 
    * getEven(null, 10); // 10,8,6,4,2,0,-2,-4 ... 
    * getEven(10, null, 2); // 10,6,2, -2 ... 
    * getEven(10,20); // 10,12,14,16,18,20 
    * getEven(20,10); // 20,18,16,14,12,10 
    * getEven(10,20,2); // 10,14,18 
    * 
    * @param int|null $start 
    * @param int|null $end 
    * @param int $step 
    * @throws InvalidArgumentException|LogicException 
    * @return Generator 
    */ 
    public function getEven($start = null, $end = null, $step = 1) 
    { 
     // Throws LogicException 
     $this->throwExceptionIfAllNulls([$start, $end]); 
     $this->throwExceptionIfInvalidStep($step); 

     // Throws InvalidArgumentException 
     $this->throwExceptionIfNotNullOrInt([$start, $end]); 

     // infinite increase range 
     if(is_int($start) && is_null($end)) 
     { 
      // throw LogicException 
      $this->throwExceptionIfOdd($start); 

      $Generator = function() use ($start, $step) 
      { 
       for($i = $start; true; $i += $step * 2) 
       { 
        yield $i; 
       } 
      }; 
     } 
     // infinite decrease range 
     elseif(is_int($end) && is_null($start)) 
     { 
      // throws LogicException 
      $this->throwExceptionIfUneven($end); 

      $Generator = function() use ($end, $step) 
      { 
       for($i = $end; true; $i -= $step * 2) 
       { 
        yield $i; 
       } 
      }; 
     } 
     // predetermined range 
     else 
     { 
      // throws LogicException 
      $this->throwExceptionIfUneven($start); 
      $this->throwExceptionIfUneven($end); 

      // decrease 
      if($start >= $end) 
      { 
       $Generator = function() use ($start, $end, $step) 
       { 
        for($i = $start; $i >= $end; $i -= $step * 2) 
        { 
         yield $i; 
        } 
       }; 
      } 
      // increase 
      else 
      { 
       $Generator = function() use ($start, $end, $step) 
       { 
        for($i = $start; $i <= $end; $i += $step * 2) 
        { 
         yield $i; 
        } 
       }; 
      } 
     } 

     return $Generator(); 
    } 

類也有一個名爲getOdd方法(是的,它看起來很多喜歡它;))

主要dublication是關閉$Generator = function() ...,差異主要是運營商如+ - * /和參數的for循環。這在其他階段基本相同。

我讀Dynamic Comparison Operators in PHP並得出的結論是,有像compare(...)

我應該做一個私人/受保護的方法比較原始方法。如果是這樣,我應該爲此創建一個新的類/函數?我不認爲它屬於當前班級。

這是別的我錯過了,我不確定如何以適當的方式幹這件事?

Btw。我知道一個getEven,getOdd有點傻時,我得到了一個getRange使用step函數,但它是一個更一般的重構/模式問題。

更新 @github的getEven和getOdd正在刪除...

回答

1

低於沒有經過測試或驗證工作,但我有信心它和它至少顯示了一種可能的方式代碼刪除多個生成器函數。

正如你自己說的,你試圖去除的重複主要在生成器函數中。如果你看看這個,你可以看到你擁有的每發生器功能可以寫成這樣:

function createGenerator($index, $limit, $step) { 
    return function() use($index, $limit, $step) { 
     $incrementing = $step > 0; 
     for ($i = $index; true; $i += 2 * $step) { 
      if (($incrementing && $i <= $limit) || (!$incrementing && $i >= $limit)) { 
       yield $i; 
      }else { 
       break; 
      } 
     } 
    }; 
} 

爲了利用這一點,你需要做一些魔術的輸入參數,它有助於(至少使它漂亮)來定義一些常量。 PHP已經獲得了一個PHP_INT_MAX常量,它保​​留了一個整數可能的最大值,但它沒有得到PHP_INT_MIN。所以我會將其定義爲自己的常數。

define('PHP_INT_MIN', ~PHP_INT_MAX); 

現在讓我們來看看你的函數中的四種情況。

1)無限增加範圍

Infinte是一個相當大膽的主張在這裏,如果我們改變它,我們得到一個有限的範圍$indexPHP_INT_MAX,因此「給int的約束儘可能大的價值」通過設置$limit = PHP_INT_MAX;,上述發電機功能仍然是相同的。

//$limit = PHP_INT_MAX; 
createGenerator($index, PHP_INT_MAX, $step); 

2)無限減少幅度

如上可以再次在這裏使用,但具有一個是負面$step和交換$index$limit同樣的論點;

//$index = $limit; 
//$limit = PHP_INT_MIN; 
//$step *= -1; 
createGenerator($limit, PHP_INT_MIN, -1 * $step); 

3)預定減小範圍

交換,並再次否定。

//$temp = $index; 
//$index = $limit; 
//$limit = $temp; 
//$step *= -1; 
createGenerator($limit, $index, -1 * $step); 

4)預定範圍日益擴大

嗯,這只是默認情況下,所有的參數都給出。沒有什麼需要改變的。

createGenerator($index, $limit, $step); 

修改後的代碼

public function getEven($index = null, $limit = null, $step = 1) { 
    // Throws LogicException 
    $this->throwExceptionIfAllNulls([$index, $limit]); 
    $this->throwExceptionIfInvalidStep($step); 

    // Throws InvalidArgumentException 
    $this->throwExceptionIfNotNullOrInt([$index, $limit]); 

    //Generator function 
    function createGenerator($index, $limit, $step) { 
     return function() use($index, $limit, $step) { 
      $incrementing = $step > 0; 
      for ($i = $index; true; $i += 2 * $step) { 
       if (($incrementing && $i <= $limit) || (!$incrementing && $i >= $limit)) { 
        yield $i; 
       }else { 
        break; 
       } 
      } 
     }; 
    } 
    // infinite increase range 
    if (is_int($index) && is_null($limit)) { 
     // throw LogicException 
     $this->throwExceptionIfodd($index); 
     return createGenerator($index, PHP_INT_MAX, $step); 
    } 
    // infinite decrease range 
    elseif (is_int($limit) && is_null($index)) { 
     // throws LogicException 
     $this->throwExceptionIfodd($limit); 
     return createGenerator($limit, PHP_INT_MIN, -1*$step); 
    } 
    // predetermined range 
    else { 
     // throws LogicException 
     $this->throwExceptionIfodd($index); 
     $this->throwExceptionIfodd($limit); 

     // decrease 
     if ($index >= $limit) { 
      return createGenerator($limit, $index, -1 * $step); 
     } 
     return createGenerator($index, $limit, $step); 
    } 
} 
+0

+1謝謝你的偉大的答案。這裏有幾個問題/註釋。答:請重新考慮「$ incrementing = $ step> 0;」在發生器內部,發生器不應包含該邏輯。 B.我想避免定義PHP_INT_MAX當我不能做「const PHP_INT_MIN =〜PHP_INT_MAX」,因爲希望代碼暴露不隱藏實現。我將主動測試重構代碼。但一個很好的答案。 –

+0

通過代碼運行後,我改變了一些小的位,如使用INF而不是PHP_INT_MAX(和-INF)。將$增量值移到生成器外部。我還重構了getEven和getOdd,因爲它們在getRange存在時很愚蠢(如上所述)。我會盡快給予獎勵。謝謝 –

+0

我絕對同意你關於'$ incrementing'的一步。我建議將它從生成器中移出,但仍保留在'createGenerator'函數中。例如。 '函數createGenerator(args){incrementing;返回發生器; }'。但我並不完全確定你的意思是「想要揭露不隱藏實現」,但是你認爲正確的是由你決定的。請記住整數的溢出和下溢可能會發生,並且可能會造成混亂。 – atomman