🐫 ⤵️ 🖐🏻 たぶん「クリーンコード」をすすめる ⤵️ 🦆 👨🏼‍🚒

「良いコード」または「クリーンなコード」の経験的な定義に到達することは決してできないかもしれません。これは、ある人の別の人の「クリーンなコード」の意見に対する意見は、必然的に非常に主観的であることを意味します。 Robert Martin の 2008年の本、Clean Codeを他人の視点から見ることはできません。

しかし、私にとってこの本の主な問題は、その中のコード例の多くがひどいことです。

第3章の「関数」では、Martinは優れた機能を作成するためのさまざまなヒントを提供しています。おそらく、この章で最も強力なヒントは、関数が抽象化のレベルを混合してはならないということです。高レベルまたは低レベルのタスクを実行するべきではありません。機能の責任を混乱させ、混乱させるためです。この章には他にも重要な点があります。マーティンは、関数名は説明的で一貫性があり、動詞句でなければならず、慎重に選択する必要があると述べています。彼は、関数は1つのことだけを実行する必要があり、それをうまく実行すると述べています。彼は、関数には副作用があってはならず（そして、彼は本当に素晴らしい例を挙げています）、戻り値を優先して出力引数を避けるべきだと述べています。彼は関数は通常何かをするコマンドのどちらかであるべきだと言っています、または何かのためにそれを要求することによってと答えますが、一度に両方ではありません。彼はDRYに説明する。少し表面的で基本的なものですが、これらはすべて良いアドバイスです。

しかし、この章にはもっと疑わしい声明があります。マーティンはブールフラグ引数は、私は飾り気のないために同意した、悪い習慣であることを述べているtrueか、falseソースコードで不透明であると明示的なものに比べて不明瞭IS_SUITEかIS_NOT_SUITE、...しかし、マーティンの推論は、ブール引数手段の機能がよりないということではなくということです彼女がしてはいけないことよりも。

マーティンは、説明として単一のソースファイルを上から下に読むことが可能であるべきだと述べ、各関数の抽象化のレベルは、それが読み取られるにつれて減少し、各関数は他の関数をさらに参照します。これは普遍的ではありません。多くのソースファイルは、ほとんどのソースファイルですが、このようにきちんと階層化することはできません。また、可能な場合でも、IDEを使用すると、Webサイトを閲覧するのと同じように、関数呼び出しから関数実装に、またはその逆に簡単にジャンプできます。また、コードを上から下に読んでいますか？まあ、多分私たちの一部はそれを行います。

そして、それは奇妙になります。マーティンは、関数はネストされた構造（条件とループ）を含むのに十分な大きさにすべきではないと述べています。 2つ以上のレベルでインデントしないでください。彼は、ブロックは1行の長さでなければならず、おそらく1つの関数呼び出しで構成されるべきだと述べています。彼は、理想的な関数には引数がない（ただし副作用はない??）と述べ、3つの引数を持つ関数は混乱しやすく、テストが難しいと述べています。奇妙なことに、マーティンは理想的な関数は2行または4行のコードであると主張しています。このアドバイスは、実際には章の最初に置かれています。これは最初で最も重要なルールです。

: . : . . , , . , . 3000 . 100 300 . 20 30 . ( ), .

[...]

Kentがコードを見せてくれたとき、すべての機能がいかにコンパクトであるかがわかりました。Swingプログラムでの私の機能の多くは、ほぼ数キロメートルにわたって垂直方向に拡張されました。ただし、ケントプログラムの各関数は、2、3、または4行しか占めていません。すべての機能はかなり明白でした。各機能には独自のストーリーがあり、各ストーリーは当然次のストーリーの始まりにあなたを導きます。これが関数の短さです。

このヒント全体は、第3章の最後にあるソースコードのリストで終わります。このコード例は、オープンソースのテストツールFitNesseに由来する、JavaクラスのMartinが好むリファクタリングです。

package fitnesse.html;

import fitnesse.responders.run.SuiteResponder;
import fitnesse.wiki.*;

public class SetupTeardownIncluder {
  private PageData pageData;
  private boolean isSuite;
  private WikiPage testPage;
  private StringBuffer newPageContent;
  private PageCrawler pageCrawler;


  public static String render(PageData pageData) throws Exception {
    return render(pageData, false);
  }

  public static String render(PageData pageData, boolean isSuite)
    throws Exception {
    return new SetupTeardownIncluder(pageData).render(isSuite);
  }

  private SetupTeardownIncluder(PageData pageData) {
    this.pageData = pageData;
    testPage = pageData.getWikiPage();
    pageCrawler = testPage.getPageCrawler();
    newPageContent = new StringBuffer();
  }

  private String render(boolean isSuite) throws Exception {
     this.isSuite = isSuite;
    if (isTestPage())
      includeSetupAndTeardownPages();
    return pageData.getHtml();
  }

  private boolean isTestPage() throws Exception {
    return pageData.hasAttribute("Test");
  }

  private void includeSetupAndTeardownPages() throws Exception {
    includeSetupPages();
    includePageContent();
    includeTeardownPages();
    updatePageContent();
  }


  private void includeSetupPages() throws Exception {
    if (isSuite)
      includeSuiteSetupPage();
    includeSetupPage();
  }

  private void includeSuiteSetupPage() throws Exception {
    include(SuiteResponder.SUITE_SETUP_NAME, "-setup");
  }

  private void includeSetupPage() throws Exception {
    include("SetUp", "-setup");
  }

  private void includePageContent() throws Exception {
    newPageContent.append(pageData.getContent());
  }

  private void includeTeardownPages() throws Exception {
    includeTeardownPage();
    if (isSuite)
      includeSuiteTeardownPage();
  }

  private void includeTeardownPage() throws Exception {
    include("TearDown", "-teardown");
  }

  private void includeSuiteTeardownPage() throws Exception {
    include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");
  }

  private void updatePageContent() throws Exception {
    pageData.setContent(newPageContent.toString());
  }

  private void include(String pageName, String arg) throws Exception {
    WikiPage inheritedPage = findInheritedPage(pageName);
    if (inheritedPage != null) {
      String pagePathName = getPathNameForPage(inheritedPage);
      buildIncludeDirective(pagePathName, arg);
    }
  }

  private WikiPage findInheritedPage(String pageName) throws Exception {
    return PageCrawlerImpl.getInheritedPage(pageName, testPage);
  }

  private String getPathNameForPage(WikiPage page) throws Exception {
    WikiPagePath pagePath = pageCrawler.getFullPath(page);
    return PathParser.render(pagePath);
  }

  private void buildIncludeDirective(String pagePathName, String arg) {
    newPageContent
      .append("\n!include ")
      .append(arg)
      .append(" .")
      .append(pagePathName)
      .append("\n");
  }
}

もう一度繰り返します。これは、マーティンの個人的な基準に従って書かれた独自のコードです。これはトレーニングの例として提示された理想です。

この時点で、私のJavaスキルは古くて錆びていることを告白します。これは、2008年に発表されたこの本と同じくらい古くて錆びています。しかし、2008年でさえ、このコードは読みにくいゴミでしたか？ワイルドカードは

無視しimportてみましょう。

2つのパブリック静的メソッド、1つのプライベートコンストラクターと15のプライベートメソッドがあります。 15個のプライベートメソッドのうち、13個には副作用があります（引数として渡されなかった変数を変更します。たとえばbuildIncludeDirective、newPageContent）、または副作用のある他のメソッドを呼び出す（たとえばinclude、を呼び出すbuildIncludeDirective）。のみisTestPageとfindInheritedPage副作用なしのように見えます。それらは（pageDataそしてtestPageそれに応じて）渡されない変数を引き続き使用しますが、副作用なしでそうするようです。

この時点で、おそらくマーティンの「副作用」の定義には、メソッドを呼び出したばかりのオブジェクトのメンバー変数が含まれていないと結論付けることができます。これらの変数の私たちはこの定義を受け入れた場合は、5、、pageData、、isSuite およびtestPagenewPageContentpageCrawler各プライベートメソッド呼び出しに暗黙的に渡され、これは正常と見なされます。プライベートメソッドは、これらの変数のいずれかを使用して自由に実行できます。

しかし、これは間違った仮定です。この章の前半からのマーティン自身の定義は次のとおりです。

副作用は嘘です。関数は1つのことを約束しますが、ユーザーから隠された別のことを行います。時には、クラスの変数に予期しない変更を加えることがあります。たとえば、関数に渡されたパラメーターの値やシステムのグローバル変数を割り当てます。いずれにせよ、そのような機能は陰湿で悪意のある嘘であり、不自然な時間バインディングやその他の依存関係の作成につながることがよくあります。

私はこの定義が好きです！私はこの定義に同意します！これは非常に便利な定義です。関数が自身のクラスの変数に予期しない変更を加えるのは悪いことだと私は同意します。

では、なぜマーティン自身のコード「クリーン」コードがこれ以外のことをしないのでしょうか。これらの信じられないほど小さなメソッドはほとんど何もせず、副作用のみで機能するため、これらのコードが何をしているかを理解するのは非常に困難です。プライベートメソッドを1つだけ見てみましょう。

private String render(boolean isSuite) throws Exception {
   this.isSuite = isSuite;
  if (isTestPage())
    includeSetupAndTeardownPages();
  return pageData.getHtml();
}

このメソッドに値を設定する副作用があるのはなぜthis.isSuiteですか？isSuiteブール値として後のメソッド呼び出しに渡すだけではどうですか。何もせpageData.getHtml()ずに3行のコードを費やした後に戻るのはなぜpageDataですか？includeSetupAndTeardownPagespageDataに副作用があると合理的に想定できますが、それではどうでしょうか。見ない限り、どちらか一方を知ることはできません。そして、これは他のメンバー変数にどのような他の副作用がありますか？不確実性が非常に大きくなるので、isTestPage副作用もあるのではないかと突然不思議に思います。（その棚は何ですか？中括弧はどこですか？）

マーティンはこの章で、関数をより小さな関数に分解することには意味があると主張しています。しかし、それから彼は私たちに与えます：

private WikiPage findInheritedPage(String pageName) throws Exception {
  return PageCrawlerImpl.getInheritedPage(pageName, testPage);
}

注意：このコードの悪い面のいくつかはマーティンのせいではありません。これは既存のコードのリファクタリングであり、明らかに彼によって最初に作成されたものではないようです。このコードには、疑わしいAPIと疑わしい動作がすでにあり、どちらもリファクタリングで存続します。まず、クラス名がSetupTeardownIncluderひどい。すべてのクラス名のように、少なくとも名詞句ですが、古典的な窒息動詞句です。これは、厳密にオブジェクト指向のコードで作業するときに必ず得られるクラス名です。すべてがクラスである必要がありますが、実際には単純な関数が1つだけ必要な場合もあります。

第二に、内容がpageData破壊されます。メンバ変数（とは異なりisSuite、testPage、newPageContentおよびpageCrawler）、我々は実際に自身のない操作を行いますpageData変更のため。最初は、外部の呼び出し元によってトップレベルのパブリックレンダラーに渡されます。レンダラーは素晴らしい仕事をして、最終的にHTML文字列を返します。ただし、この作業中は、副作用としてpageData破壊的に変更されます（を参照updatePageContent）。もちろん、PageData必要な変更を加えた完全に新しいオブジェクトを作成し、元のオブジェクトはそのままにしておくのが望ましいでしょうか？呼び出し側がそれpageDataを別の目的で使用しようとすると、その内容に何が起こったかに非常に驚かれる可能性があります。しかし、これは、マーティンがリファクタリングする前の元のコードの動作とまったく同じです。彼は非常に効果的にそれを埋めたが、彼はこの行動を保持しました。

*

本全体はそのようなものですか？

主にそうです。 Clean Codeは、非常に疑わしい、または時代遅れの強力で時代を超越したヒントとアドバイスの無害な組み合わせを組み合わせています。本はほとんどオブジェクト指向のコードに焦点を当て、SOLIDの長所を求めています他のプログラミングパラダイムを除外します。他のプログラミング言語、さらには他のオブジェクト指向言語を除いて、Javaコードに焦点を当てています。 SmellsとHeuristicsに関する章があります。これは、コードで探すかなり合理的な手がかりのリストに過ぎません。しかし、Javaコードのリファクタリングの時間のかかる、よく試された例に焦点を当てた、ほとんど空の章がいくつかあります。 JUnitの内部を探る章全体があります（この本は2008年に書かれたので、これがどれほど関連性があるか想像できるでしょう）。この本でのJavaの一般的な使用法は非常に古いものです。これらの種類のことは避けられません-プログラミングの本は伝統的にすぐに時代遅れになります-しかし、その時でさえ、提供されたコードは悪いです。

ユニットテストに関する章があります。この章には、基本的ではありますが、ユニットテストを高速、独立、再現可能にする方法、ユニットテストでソースコードをより確実にリファクタリングする方法、およびユニットテストがほぼ同じ長さであることに関する情報がたくさんあります。テスト中のコードに似ていますが、読みやすく、理解しやすくなっています。次に、著者はユニットテストを示していますが、彼によると、詳細が多すぎます。

@Test
  public void turnOnLoTempAlarmAtThreashold() throws Exception {
    hw.setTemp(WAY_TOO_COLD);
    controller.tic();
    assertTrue(hw.heaterState());
    assertTrue(hw.blowerState());
    assertFalse(hw.coolerState());
    assertFalse(hw.hiTempAlarm());
    assertTrue(hw.loTempAlarm());
  }

そして誇らしげにそれを作り直します：

@Test
  public void turnOnLoTempAlarmAtThreshold() throws Exception {
    wayTooCold();
    assertEquals(“HBchL”, hw.getState());
  }

これは、テスト用に新しいドメイン固有のテスト言語を発明することの価値に関する一般的なレッスンの一部として行われます。私はこの発言にとても戸惑いました。私はまったく同じコードを使用して、まったく反対のアドバイスを示します！やめてください！

*

著者はTDDの3つの法律を提示します。

. , .

. , . .

. , .

, , , 30 . , .

...しかし、マーティンは、プログラミングタスクを小さな32秒の部分に分割することは、ほとんどの場合めちゃくちゃに時間がかかり、多くの場合明らかに役に立たず、多くの場合不可能であるという事実に注意を払いません。

*

「オブジェクトとデータ構造」の章があり、著者はデータ構造の例を示しています。

public class Point {
  public double x;
  public double y;
}

オブジェクトのこの例（まあ、1つのオブジェクトのインターフェイス）：

public interface Point {
  double getX();
  double getY();
  void setCartesian(double x, double y);
  double getR();
  double getTheta();
  void setPolar(double r, double theta);
}

彼は書いている：

, . , . . . , , . , .

そしてそれはすべてですか？

はい、あなたはそれを正しく理解しました。マーティンの「データ構造」の定義は、他の誰もが使用する定義とは矛盾しています！本は何も言わないで純粋に私たちのほとんどは、データ構造を考えるものを使用してコーディングについてのすべてを。この章は予想よりもはるかに短く、役立つ情報はほとんど含まれていません。

*

残りのメモをすべて書き直すことはしません。それらが多すぎて、この本で私が間違っていると考えるすべてのものをリストするのに時間がかかりすぎるでしょう。別の悪質なコード例に焦点を当てます。これは、第8章の素数ジェネレータです。

package literatePrimes;

import java.util.ArrayList;

public class PrimeGenerator {
  private static int[] primes;
  private static ArrayList<Integer> multiplesOfPrimeFactors;

  protected static int[] generate(int n) {
    primes = new int[n];
    multiplesOfPrimeFactors = new ArrayList<Integer>();
    set2AsFirstPrime();
    checkOddNumbersForSubsequentPrimes();
    return primes;
  }

  private static void set2AsFirstPrime() {
    primes[0] = 2;
    multiplesOfPrimeFactors.add(2);
  }

  private static void checkOddNumbersForSubsequentPrimes() {
    int primeIndex = 1;
    for (int candidate = 3;
         primeIndex < primes.length;
         candidate += 2) {
      if (isPrime(candidate))
        primes[primeIndex++] = candidate;
    }
  }

  private static boolean isPrime(int candidate) {
    if (isLeastRelevantMultipleOfNextLargerPrimeFactor(candidate)) {
      multiplesOfPrimeFactors.add(candidate);
      return false;
    }
    return isNotMultipleOfAnyPreviousPrimeFactor(candidate);
  }

  private static boolean
  isLeastRelevantMultipleOfNextLargerPrimeFactor(int candidate) {
    int nextLargerPrimeFactor = primes[multiplesOfPrimeFactors.size()];
    int leastRelevantMultiple = nextLargerPrimeFactor * nextLargerPrimeFactor;
    return candidate == leastRelevantMultiple;
  }

  private static boolean
  isNotMultipleOfAnyPreviousPrimeFactor(int candidate) {
    for (int n = 1; n < multiplesOfPrimeFactors.size(); n++) {
      if (isMultipleOfNthPrimeFactor(candidate, n))
        return false;
    }
    return true;
  }

  private static boolean
  isMultipleOfNthPrimeFactor(int candidate, int n) {
   return
     candidate == smallestOddNthMultipleNotLessThanCandidate(candidate, n);
  }

  private static int
  smallestOddNthMultipleNotLessThanCandidate(int candidate, int n) {
    int multiple = multiplesOfPrimeFactors.get(n);
    while (multiple < candidate)
      multiple += 2 * primes[n];
    multiplesOfPrimeFactors.set(n, multiple);
    return multiple;
  }
}

このコードは何ですか？メソッドの名前は何ですか？set2AsFirstPrime？smallestOddNthMultipleNotLessThanCandidate？それはきれいなコードでなければなりませんか？素数をふるいにかける明確でインテリジェントな方法？

それがこのプログラマーが作成するコードの品質である場合-余暇に、理想的な条件で、実際の製品ソフトウェア開発のプレッシャーなしで-なぜわざわざ彼の本の残りの部分に注意を払う必要があるのでしょうか。または彼の他の本？

*

このエッセイを書いたのは、常にClean Codeを推奨している人がいるためです。私は反推薦を提供する必要性を感じました。

私は元々、職場のグループでClean Codeを読みました。週に13週間、章を読みました。

したがって、各セッションの終わりにグループが全会一致の同意のみを表明することは望ましくありません。あなたは本が読者からのある種の反応、いくつかの追加のコメントを呼び起こすことを望みます。そして、ある程度、これは、あなたがそう思うと思うように、本があなたが同意しない何かを述べなければならないか、トピックを完全に開示してはならないことを意味すると思います。これに基づいて、「クリーンコード」が適切であることが判明しました。良い議論ができました。個々の章を、現在の現代的な実践についてより深く議論するための出発点として使用することができました。本に記載されていないことをたくさん話しました。私たちは多くのことに同意しませんでした。

この本をお勧めしますか。ない。初心者向けのテキストとしても、上記のすべての警告がありますか？ない。多分2008年に私はあなたにこの本をお勧めしますか？過去の成果物として、2008年にプログラミングのベストプラクティスがどのようになっていたかの教育的なスナップショットとして、今それを推奨できますか？いいえ、しません。

*

それで、主な質問は、代わりにどの本を勧めますか？知りません。閉じない限り、コメントで提案してください。

たぶん「クリーンコード」をすすめる

More articles: