質問
PHPMDが私のコードの一部が高いNPATの複雑さを持っていると報告したときに、PHP MESS検出器でコードを分析していました。一例は次のとおりです。
function compareDates($date1, $date2){
if($date->year < $date2->year){
return -1;
}
if($date->year > $date2->year){
return 1;
}
if($date->month < $date2->month){
return -1;
}
if($date->month > $date2->month){
return 1;
}
if($date->day < $date2->day){
return -1;
}
if($date->day > $date2->day){
return 1;
}
// etc.. same for hour, minute, second.
return 0;
}
その結果、この関数は非常に高いNPATの複雑さを持っています。このような制御構造とNPATの複雑さを減らすための一般的なコーディング方法はありますか?
ソースコード: http://code.google.com/p/phpraise/source/browse/trunk/phpraise/core/datetime/raideatitime.php#546
解決
あなたのコードは実際には比較的単純で、構造が不十分です。 2つのパラメーターを取得し、-1/1の戻りを処理してから、フィールドの配列を繰り返してチェックするサブ機能を作成することをお勧めします。これは少し簡単になるので、 しかし いくつかの注意事項:
あなたのやり方は大丈夫です。それはきれいではありませんが、それがうまくいかない場合、それがそれを変更する必要はありません。
複雑さは聖杯ではありません。それは重要であり、多くのメンテナンスプログラミングを行うプログラマーとして 本当 私が維持しているコードを書いている人々が複雑さについて知っていることが重要ですが、複雑さを完全に避けることはできず、時には複雑な解決策(McCabeの複雑さを使用)が最も読みやすいです。
私が本当にあなたが作ることをお勧めする唯一の変更は、1回の返品コールをすることです。次のようなことをします:
$compare_val = 0;
ファイルの上部で、その後の[elseifsへの呼び出し]を変更し、値を返す代わりに、$ compare_valを更新して、関数の最後に返します。
他のヒント
ソート関数は-1,0,1を返す必要があるという一般的な誤解です。できるよ
function compareDates($date1, $date2)
{
return strtotime("{$date1->year}-{$date1->month}-{$date1->day}")
- strtotime("{$date2->year}-{$date2->month}-{$date2->day}");
}
整数制限が問題である場合、使用できることに注意してください DateTime
, 、その制限はありません
function compareDates($date1, $date2)
{
return new DateTime("{$date1->year}-{$date1->month}-{$date1->day}")
< new DateTime("{$date2->year}-{$date2->month}-{$date2->day}");
}
一般的にNPATの複雑さを減らすためには、可能な実行パスの数を減らす必要があります。その章をご覧ください 条件付き式を簡素化します Fowlerのリファクタリングブックから始めます。
サイドノートでは、その盛り上がったことの利点は何だろうと思いますか?ネイティブDateTime APIができないことは何でもできますか?そうでない場合、なぜそれを使いたいのですか?
PHPの初めてのimは、このコードは同じことをしていませんが、単純ですか?
function compareDates($date1, $date2){
if(($date->year < $date2->year) || ($date->month < $date2->month) || ($date->day < $date2->day) {
return -1;
}
if($date->year > $date2->year) || ($date->month > $date2->month) || ($date->day > $date2->day) {
return 1;
}
// etc.. same for hour, minute, second.
return 0;
}