最近のコード レビューで、クラス内に数行の重複したロジック (15 行未満) を見つけました。私がコードのリファクタリングを作者に提案したとき、彼はコードはその方が理解しやすいと主張しました。コードをもう一度読んだ後、重複したロジックを抽出すると可読性が少し損なわれることに同意する必要があります。
DRY はガイドラインであり、絶対的なルールではありません。しかし、一般的に、DRY の名の下に可読性を損なうことを厭いませんか?
最近のコード レビューで、クラス内に数行の重複したロジック (15 行未満) を見つけました。私がコードのリファクタリングを作者に提案したとき、彼はコードはその方が理解しやすいと主張しました。コードをもう一度読んだ後、重複したロジックを抽出すると可読性が少し損なわれることに同意する必要があります。
DRY はガイドラインであり、絶対的なルールではありません。しかし、一般的に、DRY の名の下に可読性を損なうことを厭いませんか?
3つのルール
初めて何かをするときは、ただそれをするだけです。似たようなことを 2 回目にすると、重複にひるみますが、 とにかく
重複することを行います。
3 回目に似たようなことをするときは、リファクタリングします。
ストライク 3 回でリファクタリングできます。
Seibel:つまり、これらの XII 呼び出しのそれぞれについて、実装を作成しています。
非常によく似たコードが大量に蓄積されていることに気付いたことはありませんか?ザウィンスキー:ええ、間違いなく。通常、その
コードをカット アンド ペーストした 2 回目または 3 回目までには、カット アンド ペーストをやめて、サブルーチンに入れるときが来ます。
私はどれも容認しません。時間の制約などにより、一部をいただく場合があります。しかし、複製されたコードが本当に正当化されるケースはまだ見つかっていません。
読みやすさを損なうと言うことは、名前を選ぶのが下手であることを示唆しているだけです:-)
個人的には、何よりもまず、コードを理解しやすくすることを好みます。
DRYは、コードのメンテナンスを容易にすることです。繰り返されるコードを削除するためにコードを理解しにくくすると、多くの場合、コードの行が繰り返されるよりも保守性が損なわれます。
そうは言っても、実用的な場合、DRYが従うべき良い目標であることに同意します。
問題のコードに明確なビジネスまたはテクノロジーサポートの目的Pがある場合は、通常、それをリファクタリングする必要があります。そうしないと、クローンコードに関する古典的な問題が発生します。最終的には、Pをサポートするコードを変更する必要があり、それを実装するすべてのクローンが見つかりません。
一部の人々は、3つ以上のコピーがリファクタリングのしきい値であると示唆しています。2つある場合は、そうする必要があると思います。大きなシステムで他のクローンを見つけること[またはそれらが存在する可能性があることを知っていることさえ]は、2つまたは3つ以上あるかどうかにかかわらず困難です。
現在、この回答は、クローンを見つけるためのツールがないという状況で提供されています。クローンを確実に見つけることができる場合、リファクタリング(メンテナンスエラーを回避する)の元の理由はあまり説得力がありません(名前付きの抽象化を持つことの有用性はまだ現実的です)。本当に必要なのは、クローンを見つけて追跡する方法です。それらを抽象化することは、それらを「見つける」ことができるようにする1つの方法です(見つけることを簡単にすることによって)。
クローンを確実に見つけることができるツールは、少なくとも、クローンの更新に失敗したメンテナンスエラーを防ぐことができます。そのようなツールの1つ(私は作成者です)はCloneDRです。CloneDRは、ターゲット言語構造をガイダンスとして使用してクローンを検索するため、空白のレイアウト、コメントの変更、変数の名前変更などに関係なくクローンを検索します(C、C ++、Java、C#、COBOL、PHPなどの多くの言語に実装されています)。CloneDRは、ガイダンスが提供されることなく、大規模なシステム全体でクローンを検索します。検出されたクローンとアンチユニファイアが表示されます、これは基本的に、代わりに作成した可能性のある抽象化です。そのバージョン(COBOL用)はEclipseと統合され、バッファー内のクローン内で編集しているとき、および他のクローンがどこにあるかを示し、そこにいる間に他のクローンを検査/改訂できるようになりました。(あなたがするかもしれない1つのことはそれらをリファクタリングすることです:)。
以前はクローン作成はまったく間違っていると思っていましたが、クローンが元のクローンとどのように異なるかわからないため、クローン作成が行われている時点で最終的な抽象化が明確ではないため、クローン作成を行っています。クローンを追跡でき、抽象化が明らかになった後でリファクタリングを試みることができれば、クローンは良いと思います。
何かを繰り返すとすぐに、間違いを犯した場合、それを拡張、編集、削除する必要がある場合、またはそれに遭遇する可能性のある他の何十もの理由のいずれかを編集するために複数の場所を作成しています。変更を強制します。
ほとんどの言語では、ブロックを適切な名前のメソッドに抽出しても、読みやすさが損なわれることはほとんどありません。
それはあなたの基準を備えたあなたのコードですが、あなたの「いくらですか?」に対する私の基本的な答えです。なしです...
どの言語かは言いませんでしたが、ほとんどのIDEでは、単純なリファクタリング->抽出メソッドです。それはどれほど簡単であり、いくつかの引数を持つ単一のメソッドは、重複コードの2ブロックよりもはるかに保守可能です。
リファクタリングは難しい場合があり、これは言語によって異なります。すべての言語には制限があり、複製されたロジックのリファクタリングされたバージョンは、繰り返されるコードよりも言語的に複雑になる場合があります。
多くの場合、コードLOGICの重複は、基本クラスが異なる2つのオブジェクトの動作方法が類似している場合に発生します。たとえば、両方とも値を表示するが、その値にアクセスするための共通のインターフェースを実装していない2つのGUIコンポーネント。この種のシステムをリファクタリングするには、必要以上にジェネリックオブジェクトを取得し、その後に型チェックとキャストを行うメソッドが必要です。そうでない場合は、クラス階層を再考して再構築する必要があります。
この状況は、コードが正確に複製された場合とは異なります。同じ関数内で2回だけ使用することを意図している場合は、必ずしも新しいインターフェイスクラスを作成する必要はありません。
抽象的に言うのは非常に難しい。しかし、私自身の信念は、重複するコードの 1 行でも関数にする必要があるということです。もちろん、私自身が常にこの高い基準を達成できるわけではありません。
重複コードは受け付けません。何かが複数の場所で使用されている場合、それはフレームワークまたは少なくともユーティリティ ライブラリの一部になります。
最良のコード行は、書かれていないコード行です。
DRYのポイントは保守性です。コードを理解するのが難しい場合、保守するのが難しいので、リファクタリングが読みやすさを損なう場合、実際にはDRYの目標を達成できない可能性があります。15行未満のコードについては、クラスメートに同意する傾向があります。
一般的に、いいえ。とにかく読みやすさのためではありません。複製されたコードをリファクタリングして、本、IMO のように読める一般的な方法を明らかにする方法が常にあります。
依存関係の導入を避けるために DRY に違反しているという議論をしたい場合、それはより重要になる可能性があり、Ayende の意見とコードをここで説明するコードを得ることができます。
あなたの開発者が実際に Ayende でない限り、私は DRY に固執し、意図を明らかにする方法で可読性を得るでしょう。
BH
それは実際には多くの要因、コードの使用量、読みやすさなどに依存します。この場合、コードのコピーが1つだけで、この方法で読みやすい場合は、問題ない可能性があります。しかし、同じコードを3番目に使用する必要がある場合は、それを共通の関数にリファクタリングすることを真剣に検討します。
可読性は、コードが持つことができる最も重要なことの 1 つであり、私はそれについて妥協したくありません。複製されたコードは悪臭であり、大罪ではありません。
そうは言っても、ここに問題があります。
このコードが偶然に同じであるのではなく、同じであると想定されている場合、保守性のリスクがあります。それぞれの場所に他の場所を指すコメントがあり、それが 3 番目の場所にある必要がある場合は、リファクタリングします。(実際には、適切なコード ファイルを共有しない 2 つの異なるプログラムにこのようなコードがあるため、各プログラムのコメントは他のプログラムを指しています。)
線が首尾一貫した全体を形成し、簡単に説明できる何らかの機能を実行するかどうかについて、あなたは言いませんでした. もしそうなら、それらをリファクタリングしてください。コードが 2 つの場所に埋め込まれている方が読みやすいことに同意するため、これは当てはまりません。ただし、より大きな類似性またはより小さな類似性を探し、コードを単純化するために関数を除外することもできます。数十行のコードが繰り返されているからといって、関数がその数十行だけで構成されるべきであるとは限りません。