はい、現在このプロジェクトに取り組んでいるオフィスで、この「プログラマー」がいまだにうろついていると想像してください。 私はこの話を掘り下げたくありませんが、5k行の長さの機能がプログラムの中心であり、約150k行のサイズであることに言及したいと思います。 そのひどい機能のために、プログラムの開発は最終的に停止し、アプリケーションのアーキテクチャに非常に悪い影響を与えました。 最終的に、アプリケーションを最初から書き直すことにしました。
このストーリーは、悲惨な結果をもたらした関数サイズの問題の極端な例を示しています。 もう1つの極端な方法は、脳をオフにし、内部に単一行関数を含むクラスの割り当てを開始することです。 私はそのような機能が悪いというわけではありません。あなたの脳の力を使うことを忘れてはならないという事実について話しているのです。 最初に問題を解決する必要があります。
問題をさらに詳しく調査し続ける前に、一般的に言えば、この問題に関してボブおじさんとクリスティン・ゴーマンの間で小さな戦いがあったことに注意したいと思います。 ボブおじさんは、「ドロップするまで抽出する」というテクニックを導入しました。これは、要するに、何かを抽出するまで関数を抽出することを意味します。 クリスティーンゴーマンはこのテクニックを脳の使用を排除すると考えました 。 さらに、.NET BCLから1つの関数をリファクタリングすることに関するJohn Sonmezの投稿がありました（ただし、この記事の本来の目的は、ほとんどのコメントが悪であることを示すことでした）。
ジョンによるリファクタリングの例を見てみましょう。 彼は例として次の方法を取りました。
internal static void SplitDirectoryFile( string path, out string directory, out string file) { directory = null; file = null; // assumes a validated full path if (path != null) { int length = path.Length; int rootLength = GetRootLength(path); // ignore a trailing slash if (length > rootLength && EndsInDirectorySeparator(path)) length--; // find the pivot index between end of string and root for (int pivot = length - 1; pivot >= rootLength; pivot--) { if (IsDirectorySeparator(path[pivot])) { directory = path.Substring(0, pivot); file = path.Substring(pivot + 1, length - pivot - 1); return; } } // no pivot, return just the trimmed directory directory = path.Substring(0, length); } return; }
このコードを読みやすくするために、Johnはリファクタリングされたソースメソッドを配置して新しいクラスを作成しました。 彼がしたことは次のとおりです。
public class DirectoryFileSplitter { private readonly string validatedFullPath; private int length; private int rootLength; private bool pivotFound; public string Directory { get; set; } public string File { get; set; } public DirectoryFileSplitter(string validatedFullPath) { this.validatedFullPath = validatedFullPath; length = validatedFullPath.Length; rootLength = GetRootLength(validatedFullPath); } public void Split() { if (validatedFullPath != null) { IgnoreTrailingSlash(); FindPivotIndexBetweenEndOfStringAndRoot(); if(!pivotFound) TrimDirectory(); } } private void TrimDirectory() { Directory = validatedFullPath.Substring(0, length); } private void FindPivotIndexBetweenEndOfStringAndRoot() { for (int pivot = length - 1; pivot >= rootLength; pivot--) { if (IsDirectorySeparator(validatedFullPath[pivot])) { Directory = validatedFullPath.Substring(0, pivot); File = validatedFullPath.Substring(pivot + 1, length - pivot - 1); pivotFound = true; } } } private void IgnoreTrailingSlash() { if (length > rootLength && EndsInDirectorySeparator(validatedFullPath)) length--; } }
わあ リファクタリングが本当にコードを読みやすくするのに役立ったかどうかを決めるのはそれほど簡単ではありません。 実際には、読みにくくなっているという感覚。 以前は、有用なコメントを含む比較的小さな関数がありましたが、現在はコメントのない4つの関数を含むクラスになりました。 新しいクラスが悪いとは言いませんし、すべてのリファクタリングは悪い考えであり、リファクタリングを行ったプログラマは実行されるべきです。 まったくありません。 私はそれほど血に飢えているわけではありません。 これらの2つのコード例にはいくつかの違いがあります。 これらの違いを考慮してください。
- トップレベル関数が何をするのかを深く理解しようとしている場合、関数は最初よりも読みにくくなりました。これは、すべての関数をスキップして、各関数で何が起こっているのかを理解する必要があるためです。 それどころか、元のバージョンは簡単にすぐに目を通すことができます。
- 最上位関数を概念的に構成するものを理解しようとする場合、リファクタリングされたバージョンは読みやすくなります。関数が内部で概念的に実行する内容がすぐにわかるからです。
- 私が見る3番目の違いは、サポートのコストです。 具体的な例として、リファクタリングされたバージョンをサポートするコストは最初のものよりも高いと言います（少なくともリファクタリングする必要があります）。 一般に、どのオプションをサポートするのがより高価かという質問に対する答えは、要件の面にあります。 これらの要件は、この状況でSRP（唯一の責任の原則）に従うことが重要かどうかを決定します。 この関数を作成して、時間の終わりまでに一度忘れてしまった場合、リファクタリングに時間を浪費する理由はありません。 それどころか、機能が大きくなることが予想される場合、関数を別のクラスにリファクタリングするすべての理由があります。
さらに、レガシーシステムで同様の機能に偶然（または意図的に）つまずいた状況について触れたいと思います。 すぐに4つの関数を含むクラスを抽出しようと急いでいますか？ 私のアドバイスは-コードベースのテストのカバレッジが100％になっても、理由なくこれをしないでください。 なんで？ 技術的な負債がないからです。 私は苦しみを引き起こす深刻な技術的負債について話している。
したがって、テクニックを落とすまで、抽出には何も問題はありません。 私の意見では、いくつかの考慮事項に留意する必要があります。
要約すると、意味のないアクションを決して実行しないでください。 最初に考え、分析し、結論を出してから行動する必要があります。