関数の抽出について考えたこと🤔

はじめに

現在マーチン・ファウラー氏の「リファクタリング 第2版」を読んでおり、

今回はその中でも「関数の抽出」について記事にしてみようと思います。

なお本記事はJavaScriptで書かれている上記書籍のコードをDartに置き換えた

ものになります。

本記事の趣旨により一部のロジックは簡略化しており、
実際のそれとは完全に一致はしていません。
この点をご了承いただければと思います。

リファクタリング前

まずは以下のコードをご覧ください。

// リファクタリング前
void printOwing(Invoice invoice) {
  double outstanding = 0.0;

  for (var i = 0; i < 10; i++) {
    outstanding += 1.0;
  }

  print("************************");
  print("**** Customer Owes *****");
  print("************************");
  print("name: ${invoice.customer}");
  print("amount: $outstanding");
}

ロジックとしては極めてシンプルな関数ですね。

しかしこのようなシンプルな関数にも

「意味のまとまり」があることを確認できるかと思われます👀

例えば...

// これはバーナーを表示している
print("************************");
print("**** Customer Owes *****");
print("************************");

これと、

// これは請求書の合計金額を計算している
double outstanding = 0.0;

for (var i = 0; i < 10; i++) {
    outstanding += 1.0;
}

これと、

// これは請求書の詳細を表示している
print("name: ${invoice.customer}");
print("amount: $outstanding");

これについては意味のまとまりがありそうですよね🤔

この程度の分量であれば本来リファクタリングする程でもないでしょうし、

かえって見通しが悪くなりそうな気もしますが、

productコードではこれ以上のコード量でかつ複雑でクリティカルな処理を

様々なところでしています。

何だか臭うな、嫌な予感がするなと言ったコードが随所に散見してしまうと

可読性やメンテナンス性を下げてしまうことにもなりかねないので、

ここは例に倣ってリファクタリングしてみようかと思います💪

リファクタリング後

そのためにも今回は先述の意味のまとまりをそれぞれ関数として抽出して、

それを呼び出すように書き換えてみようかと思います！

なお先に完成図を共有した方が筋道を追いやすいかと思われますので、

完成図から先に共有いたします。

// リファクタリング後
void printOwing(Invoice invoice) {
  // バーナー表示のローカル関数
  void printBanner() {
    print("************************");
    print("**** Customer Owes *****");
    print("************************");
  }

  // 請求書の合計金額を計算するローカル関数
  double getOutstanding() {
    double outstanding = 0.0;

    for (var i = 0; i < 10; i++) {
      outstanding += 1.0;
    }

    return outstanding;
  }

  // 請求書の詳細を表示するローカル関数
  void printDetails(double outstanding) {
    print("name: ${invoice.customer}");
    print("amount: $outstanding");
  }

  // バーナーを表示
  printBanner();

  // 請求書の合計金額を取得
  double outstanding = getOutstanding();

  // 請求書の詳細を表示
  printDetails(outstanding);
}

この完成図に至るためには以下の手順を踏みます。
なお、以降の後続の手順は先行の手順がpassしていることを満たした時に実行します。

バーナー表示部分のリファクタリング

• バーナー表示の部分を関数として抽出
• それを呼び出すようにコードを修正
• 修正ができたらテストをして、処理がこけないか検証

請求書の合計金額の計算部分のリファクタリング

• 請求書の合計金額の計算部分を関数として抽出
• それを呼び出すようにコードを修正
• 修正ができたらテストをして、処理がこけないか検証

請求書の詳細に関する表示部分のリファクタリング

• 請求書の詳細の表示に関する部分を関数として抽出
• それを呼び出すようにコードを修正
• 修正ができたらテストをして、処理がこけないか検証

少し冗長な構成にも感じますが、

「小さく修正を行い、その度にテストをすることで変更によるリスクを最小化しつつ、

これを積み重ねていくことで着実にコードの品質を上げていく。

（場合によってはテストの度にコミットをする）」

これが安全にリファクタリングをする上で重要なのでしょうね。

この手順に倣い、

まず最初に「バーナー表示部分」からリファクタリングをしてみましょう。

// バーナー表示の部分をリファクタリング
void printOwing(Invoice invoice) {

  // バーナー表示のローカル関数
  void printBanner() {
    print("************************");
    print("**** Customer Owes *****");
    print("************************");
  }
  
  double outstanding = 0.0;

  for (var i = 0; i < 10; i++) {
    outstanding += 1.0;
  }

  // バーナーを表示
  printBanner();
  
  print("name: ${invoice.customer}");
  print("amount: $outstanding");
}

この段階でテストを行い、passしたことを確認できたら

次のステップである「請求書の合計金額の計算部分」に進みます。

// 請求書の合計金額の計算部分もリファクタリング
void printOwing(Invoice invoice) {

  // バーナー表示のローカル関数
  void printBanner() {
    print("************************");
    print("**** Customer Owes *****");
    print("************************");
  }
  
  // 請求書の合計金額を計算するローカル関数
  double getOutstanding() {
    double outstanding = 0.0;

    for (var i = 0; i < 10; i++) {
      outstanding += 1.0;
    }

    return outstanding;
  }

  // バーナーを表示
  printBanner();

  // 請求書の合計金額を取得
  double outstanding = getOutstanding();
  
  print("name: ${invoice.customer}");
  print("amount: $outstanding");
}

この段階でもテストをpassしたら、

最後に「請求書の詳細に関する表示部分」についてリファクタリングをしましょう。

// 請求書の詳細に関する表示部分もリファクタリング
// リファクタリング完了！
void printOwing(Invoice invoice) {
  // バーナー表示のローカル関数
  void printBanner() {
    print("************************");
    print("**** Customer Owes *****");
    print("************************");
  }

  // 請求書の合計金額を計算するローカル関数
  double getOutstanding() {
    double outstanding = 0.0;

    for (var i = 0; i < 10; i++) {
      outstanding += 1.0;
    }

    return outstanding;
  }

  // 請求書の詳細を表示するローカル関数
  void printDetails(double outstanding) {
    print("name: ${invoice.customer}");
    print("amount: $outstanding");
  }

  // バーナーを表示
  printBanner();

  // 請求書の合計金額を取得
  double outstanding = getOutstanding();

  // 請求書の詳細を表示
  printDetails(outstanding);
}

これもテストがpassするか否かを検証しましょう。

passしていればリファクタリングは完了です🎉

まとめ

今回、関数の抽出について記事をまとめている時に考えたことですが、

「小さく初めて、少しずつ確実に成功させる」

といった姿勢が重要なのではないかと感じました。

一気に進めたくなる気持ちもありますが、

コード量が長大だと誤った案を講じてしまったり、

それにより出戻りが発生してしまって、

どこからどこまでが上手くいっていたのかを掴みづらくしてしまって、

結果としてリファクタリングに失敗してしまう（リファクタリングを面倒に感じてしまう）、

といったことに陥るのだと思われました。

一気にやりたい気持ちを抑えて、

焦らずゆっくり丁寧に、少しずつ確実に進めることが重要なんでしょうね🤔

また今回は原著に倣ってローカル関数で抽出してみましたが、

これについては一長一短な部分もあるかと思います。

トップレベルやプライベートな関数ではなくローカル関数にすることで

より参照元を狭めることもできますが、

この場合はコンポーネントテストを直接することは出来ず、

ローカル関数を呼び出している外側の関数を通じてテストすることになりますので、

試験性の観点からすると良くないかも知れません。

抽出した関数が複雑なロジックを持ち、クリティカルな処理であれば、

トップレベルやプライベートな関数として切り出したり、

または設計を見直してみるなど、

別の案を試みた方が良いかも知れません。

またローカル関数として抽出した場合は副作用の存在やスコープの範囲について

事前に認知して、それが安全で妥当かどうかを考える必要もあるかと思われます。

先述の通り、特にこれが複雑性を帯びていてクリティカルな処理であればあるほど、

その選択がベストなのかをよく考え、最適な案を講じる必要があるかと思われます。

一言に関数の抽出と言っても、とても奥深いものなのだと感じました。。。🧐

参考

リファクタリング 第2版