refactoring - クロスするときに常にリファクタリングするコーディングのアンチパターンの種類は何ですか?

Question

私が取り組んでいたクラスの別のセクションにあるコードをリファクタリングしたのは、一連のネストされた条件演算子 (?:) であり、非常に単純な switch ステートメント (C#) によって非常に明確になったためです。

より明確にするために、直接取り組んでいるものではないコードに触れるのはいつですか?

score 16 · Accepted Answer

私はかつてリファクタリングをしていて、次のコードのようなものに出くわしました:

string strMyString;
try
{
  strMyString = Session["MySessionVar"].ToString();
}
catch
{
  strMyString = "";
}

Resharper は .ToString() が冗長であると指摘したので、それを取り出しました。残念ながら、それはコードを壊してしまいました。MySessionVar が null の場合は常に、コードが依存していた NullReferenceException が原因で、catch ブロックにバンプされませんでした。私は知っています、これはいくつかの悲しいコードでした。しかし、私はそこから良い教訓を学びました。リファクタリングを支援するツールに頼って古いコードを手早く調べないでください。自分でよく考えてください。

私は次のようにリファクタリングしました：

string strMyString = Session["MySessionVar"] ?? "";

更新: この投稿は支持されており、技術的には質問への回答が含まれていないため、実際に質問に回答する必要があると考えました. （わかりました、それは私が実際にそれを夢見ていたところまで私を悩ませていました. ）

個人的には、リファクタリングの前にいくつか質問します。

1) システムはソース管理されていますか? もしそうなら、先に進んでリファクタリングしてください。何かが壊れたらいつでもロールバックできるからです。

2) 変更しようとしている機能の単体テストは存在しますか? もしそうなら、素晴らしい！リファクタリング。ここでの危険は、単体テストの存在が、その単体テストの精度と範囲を示していないことです。優れた単体テストでは、重大な変更を検出する必要があります。

3) リファクタリングしているコードを完全に理解していますか? ソース管理もテストもなく、変更しようとしているコードをよく理解していない場合、それは危険信号です。リファクタリングする前に、コードに慣れる必要があります。

3 番目のケースでは、リファクタリング中のメソッドを現在使用しているすべてのコードを実際に追跡するためにおそらく時間を費やします。コードの範囲に応じて、これは簡単な場合もあれば不可能な場合もあります (つまり、パブリック API の場合)。パブリック API になる場合は、ビジネスの観点からコードの本来の意図を理解する必要があります。

score 7 · Accepted Answer

テストがすでに実施されている場合にのみリファクタリングします。そうでない場合は、通常、テストを作成し、おそらく機能するコードをリファクタリングするのに時間をかける価値はありません。

score 6 · Accepted Answer

これは小さくてマイナーなアンチパターンですが、非常にイライラするので、見つけたらすぐに消去します。C（またはC ++またはJava）の場合

if (p)
    return true;
else
    return false;

になります

return p;

スキームでは、

(if p #t #f)

になります

とMLで

if p then true else false

になります

このアンチパターンは、ほとんどの場合、学部生によって作成されたコードにのみ見られます。私は間違いなくこれを構成していません!!

score 5 · Accepted Answer

私がそれに出くわすときはいつでも、それを変更しても問題が生じるとは思いません (たとえば、私はそれが何をするかを知っているほど十分に理解できます。たとえば、ブードゥー教のレベルが低いなど)。

score 3 · Accepted Answer

コードを変更している他の理由がある場合にのみ、わざわざ変更します。

私がどれだけそれを受け入れるかは、私が何も壊さないという自信と、コードに対する私自身の変更がどれほど広範囲に及ぶかによって異なります。

score 3 · Accepted Answer

簡単なリファクタリングのために、深くネストされた制御構造と非常に長い関数 (1 画面分以上のテキスト) をクリーンアップしようとします。ただし、正当な理由なしにコードをリファクタリングするのは良い考えではありません (特に大規模な開発者チームでは)。一般に、リファクタリングによってコードが大幅に改善されるか、重大な罪が修正される場合を除いて、私はそのままにしておくようにしています。

一言でリファクタリングするわけではありませんが、一般的なハウスキーピングの問題として、モジュールの作業を開始するときに、通常、次のことを行います。

愚かなコメントを削除する
- 関数のシグネチャが既に述べている以上のことを何も言わないコメント
- 「見た目そのまま」などの純粋な馬鹿げたコメント
- ファイルの先頭にある変更ログ (理由により、バージョン管理があります)
- コードと明らかに同期していない API ドキュメント
コメントアウトされたコードのチャンクを削除する
欠落している場合は、$Id$ などのバージョン管理タグを追加します
空白の問題を修正します (ただし、空白を変更しただけでも、diff の多くの行にあなたの名前が表示されるため、これは他の人にとって迷惑になる可能性があります)。
- 行末の空白を削除
- タブ->スペースを変更します（これが私が働いている慣例です）

score 3 · Accepted Answer

これは、単体テストの利点を示す絶好の機会です。

単体テストが実施されている場合、開発者は、奇妙に記述されたコードに出くわす可能性があるため、勇気を持って積極的にリファクタリングできます。単体テストに合格し、可読性が向上した場合は、その日の善行を行ったことになり、先に進むことができます。

単体テストを行わずに、ブードゥー語でいっぱいの複雑なコードを単純化すると、コードが壊れて、新しいバグが発生したことに気付かないという大きなリスクが生じます。そのため、ほとんどの開発者は慎重な道をたどって先に進みます。

score 1 · Accepted Answer

（おそらく）3行または4行を超える重複コードは、常にリファクタリングについて考えさせられます。また、私はコードを頻繁に移動する傾向があり、より頻繁に使用されると予測されるコードを別の場所に抽出します-独自の明確な目的と責任を持つクラス、または静的クラスの静的メソッド（通常はmy Utils。*名前空間）。

しかし、あなたの質問に答えるために、はい、コードを短くすることは必ずしもそれをうまく構造化して読みやすくすることを意味しない場合がたくさんあります。??を使用する C＃の演算子は別の例です。また、選択した言語の新機能についても考慮する必要があります。たとえば、LINQを使用すると、非常にエレガントな方法でいくつかの処理を実行できますが、非常に単純なことを非常に読みにくく、過度に複雑にすることもできます。これら2つのことを非常に慎重に検討する必要があります。最終的には、すべてが個人的な好みと、ほとんどの場合、経験に帰着します。

まあ、これは別の「それは依存する」答えですが、私はそれがなければならないのではないかと心配しています。

score 1 · Accepted Answer

私はほとんどの場合、2 つ以上の同様の条件をスイッチに分割します...ほとんどの場合、列挙型に関してです。長いステートメントの代わりにリターンを短くします。

元：

if (条件) {
  //大量のコード
  //戻り値
} そうしないと {
  null を返します。
}

になります：

if (!条件)
  null を返します。

//たくさんのコード..
//戻り値

早期に分割すると、余分なインデントが減り、長いコードが減ります...また、原則として、10〜15行を超えるコードを含むメソッドは好きではありません。内部でよりプライベートなメソッドを作成する場合でも、メソッドが単一の目的を持つことが好きです。

score 1 · Accepted Answer

通常、コードを参照しているだけで、積極的に作業していない場合は、コードをリファクタリングしません。

しかし、ReSharper は、Alt+Enter に抵抗できないことを指摘することがあります。:)

score 1 · Accepted Answer

リファクタリングによってコードが読みやすくなった場合、私にとって最も一般的なのは重複したコードです。たとえば、最初と最後のコマンドだけが異なる if/else です。

if($something) {
  load_data($something);
} else {
  load_data($something);
  echo "Loaded";
  do_something_else();
}

score 0 · Accepted Answer

ブロックへの入力と出力のセットを理解している場合、私は非常に長い関数とメソッドをリファクタリングする傾向があります。

これは読みやすさを際限なく助けます。

score 0 · Accepted Answer

明らかに間違っている、または明らかに無意味なコメントの削除/更新。
それらを削除することは次のとおりです。

安全な
バージョン管理は、それらを再び見つけることができることを意味します
他の人やあなた自身へのコードの品質を向上させます

それは私が知っている唯一の100％リスクのないリファクタリングです。

javadocコメントのような構造化コメントでこれを行うことは別の問題であることに注意してください。これらを変更することはリスクがないわけではありません。そのため、修正/削除される可能性は非常に高いですが、標準の誤ったコードコメントのように特定の保証された修正が無効になることはありません。

score 0 · Accepted Answer

そのためのリファクタリングは、すべての悪の根源の1つです。絶対にしないでください。（ユニットテストはこれをいくらか軽減します）。

score 0 · Accepted Answer

コメントアウトされた大量のコードはアンチパターンですか? 具体的なコードではありませんが (コメントです)、ソース管理の使用方法を理解していない人や、後で元のコードに戻って簡単に元に戻せるように古いコードを保持したい人に、この種の動作が多く見られます。バグが導入されました。コメントアウトされた大量のコードを目にするたびに、ほとんどの場合、それらを完全に削除します。

score 0 · Accepted Answer

次の要因に基づいてリファクタリングを試みます。

何が起こっているかを知るのに十分な理解がありますか?
この変更によりコードが壊れた場合、簡単に元に戻せますか?
ビルドが壊れた場合、変更を元に戻すのに十分な時間がありますか。

また、十分な時間があれば、リファクタリングして学習することもあります。のように、変更によってコードが壊れる可能性があることはわかっていますが、どこでどのように変更するかはわかりません。だから私はとにかくそれを変更して、どこが壊れているのか、その理由を見つけます。そうすれば、コードについてもっと学ぶことができます。

私のドメインはモバイルソフトウェア (携帯電話) であり、ほとんどのコードは私の PC にあり、他のコードに影響を与えることはありません。私は会社の CI ビルドシステムも維持しているので、リファクタリングされたコードで完全な製品ビルド (すべての電話用) を実行して、他の部分が壊れないようにすることができます。しかし、それはあなたが持っていないかもしれない私の個人的な贅沢です.

score 0 · Accepted Answer

次の手順を実行した後、遭遇し、積極的に取り組んでいないコードのみをリファクタリングします。

コードの作成者に相談して (常に可能であるとは限りません)、そのコードが何をするのかを理解してください。コードの一部が何をしているのかが明らかな場合でも、作成者が特定の方法で何かを行うことにした理由を理解することは常に役立ちます。それについて数分間話すことは、元の作成者があなたの視点を理解するのに役立つだけでなく、チーム内で信頼を築くことにもなります.
Know or find out what that piece of code is doing in order to test it after re-factoring (A build process with unit tests is very helpful here. It makes the whole thing quick and easy). Run the unit tests before and after the change and ensure nothing is breaking due to your changes.
Send out a heads up to the team (if working with others) and let them know of the upcoming change so nobody is surprised when the change actually occurs

score 0 · Accepted Answer

私は、リファクタリングのリスクが低いと見なされる場合、グローバル定数と列挙型をかなりリファクタリングする傾向があります。たぶんそうかもしれませんが、ClientAccountStatus列挙型のようなものは、GlobalConstAndEnumクラスではなく、ClientAccountクラス内またはその近くにある必要があります。

refactoring - クロスするときに常にリファクタリングするコーディングのアンチパターンの種類は何ですか?

18 に答える 18

Related

Reference