- 小さな関数に分けて抽象化レベルを分離する方法よりも、上から下へ続く線形コードのほうが全体の流れを追いやすいという立場
- 関数抽出によってトップダウン（top-down）構造を作ると、
bake と bakePizza のように名前が似た関数の間を行き来して確認しなければならない場合がある
- オーブンの予熱位置や、ピザを2回渡したときの結果のように、小さな関数は意図を示す代わりに実際の動作を隠すことがある
- 線形コードに段階ごとのコメントを付ければ、間接参照を増やさずに作業の意図を説明できるため、追加の抽象化より読みやすい場合がある
- 一度しか使わない小さな関数を抽出すると線形性の喪失が生じ、例のオーブン生成方法のように、実際のコードでは性能問題まで露呈することがある
関数抽出より線形性が重要な場合
- Google Testing Blog の例は2つの
createPizza 実装を比較し、右側の実装は抽象化レベルを混ぜていないため読みやすく、トップダウン（top-down） だと見なしている
- 反対の立場では、左側の実装が画面の上から下へ線形に読めるコードである点のほうが重要だとする
- 右側の実装は全体の動作を理解するために、複数の小さな関数定義へ移動しなければならない
- 表示方法でも右側コードの一部が省略されており、2つの実装の大きさが似て見えるが、実際には右側のほうが長い
- 関数抽出は、名前だけでは動作を十分に把握しにくくすることがある
bake と bakePizza がどちらもあると、どちらの関数がオーブンを温めるのかすぐには分かりにくい- 同じピザを2回渡したときに冪等なのか、それとも結果が壊れるのかを確認するには、内部実装を見る必要がある
コメント付きの線形コードとオーブンの例
- 左側の線形コードに右側の関数名をコメントとして付けた版が、最も読みやすい形だと評価されている
Prepare pizza、Add toppings、Heat oven、Bake pizza、Box and slice のようなコメントが各段階の意図を示す- 可読性は追加の抽象化レイヤーや間接参照ではなく、今行っていることを適切に説明するところから生まれる
- 結論は、一度しか使わない小さな関数を線形コードから切り出すべきではない、という方向に近い
- 小さな関数抽出の利点は線形性の喪失を相殺できないと見ている
- 例のオーブン処理も構造的に不自然
- オーブンの予熱はそれ自体で完結した動作なので、オーブンのメソッドにするほうが適切
- ピザを1枚作るたびに新しいオーブンを作って予熱する流れは、現実的な使い方に合っていない
- 実際のコードでもこうした構造は現れ、ときには性能問題を引き起こすことがある
- オーブンは
createPizza の内部で新しく作るよりも、引数として受け取るべき可能性が高い
- オーブンを提供するのは呼び出し側の責務に近い
- ピザを箱に入れる流れなら、ピザではなく箱を返すインターフェースのほうが自然な場合がある
1件のコメント
Hacker News の意見
スタイルの問題であり、料理と同じように塩が多すぎても少なすぎても料理は台無しになる。
ここで誰かが1000行の神関数を1つ提案しているわけではないことを願うし、1関数あたり最大5行というのも読みやすくはない。どこで分けるかには判断力とよい感覚、反復が必要だ。最初に試した抽象化がいまひとつだったからといって抽象化を諦めるべきではなく、何度かリファクタリングしていくうちに、ビジネスドメインとうまく合うクラスや API が出てくることもある。
同時に、抽象化にあまりに性急だったり、数行の重複で致命傷を負ったかのように振る舞ったりしてはいけない。性急な抽象化は、一緒に進化する必要のないコードをまとめてしまいがちだ。1か所からしか呼ばれない関数を切り出して作業単位を隠すやり方は、アルゴリズムをすっきりさせられるし、特にボイラープレートや、ビジネスロジックと DB 接続処理のようなインフラ上の関心事が混ざっている部分を隠すときに有用だ。ただし慎重に使うべきで、同じ抽象化レベルにあるべきステップを分断するのは避けたほうがよい。
一緒に働いたある人は「読みやすい」という理由であらゆるブール条件を関数に切り出し、「コメントは悪い」という理由でコメントをまったく書かなかった。その本は、こうした悪い助言を盲目的に従う狂信者を生むので嫌いだ。
ときにはドメインが1000行の神関数を要求することもあり、ロジックと作業が1か所にまとまっているため、50行の関数20個よりはるかに読みやすいこともある。結局、全体を理解するにはその20個をすべて読む必要があるし、誰かがその一部を再利用しようとして、本来の作業にはなかった2〜3の要件に合わせて調整し、特定のロジックを無関係なユースケースと結びつけてしまうこともある。
その関数が純粋関数なら、1000行だろうが10000行だろうが関係なく、依然として問題ないと思う。
料理のレシピもすでにかなり抽象化されている。「玉ねぎを軽く炒める」と言えば、玉ねぎの切り方と軽く炒めるアルゴリズムはすでに知っていると仮定している。すべてをインラインで書くと読めなくなる。
コードも似ている。抽象化を厳格に排除すると、言語が許す最低レベルまで降りることになり、それは明らかに読みやすいコードではない。たとえば Python の
decodeメソッドの代わりに Unicode デコードを自分でやろうとすると、プログラムが実際に何をしているのか理解するのが非常に難しくなる。言語が単純で十分に検証された抽象化を提供しているから誰もそうしないだけだが、自分が単純で十分に検証された抽象化を作ってビジネスロジック全体で使うことと何が違うのか。難しいのは、誰も二度と手を入れる必要がないほどよく選ばれた抽象化を作ることだ。
今のコード行が似て見えるからといって、今後も同じであるべき、あるいは同じに保たれるべきとは限らない。「コードがほとんど重複しているから」という理由で異なる2つのユースケースを無理にまとめると、時間がたつにつれて、何も抽象化できていない抽象化になりがちだ。
ユースケースが大きく分岐すると、実装は多くのロジックを呼び出し側に押し上げるか、差異をフラグとして公開して内部に別々の実装を2つ並べることになる。前者は浅い抽象化で価値が小さく、後者は独立した2つの実装より不明確だ。
サンプルコードは単純すぎるので、線形コードのほうが読みやすいのは当然だが、その考え方はうまくスケールしない
再利用性や単体テストのしやすさも考慮する必要があり、すべてのコードを単一の関数に入れると、読んでいるコードブロックと関係があるかもしれないし、ないかもしれないローカル変数がすべてスコープ内に入ってしまい、推論がより難しくなることがある
ただ、経験が浅かった頃を振り返ると、問題のない線形コードを過度にモジュール化して、あちこち飛び回らなければならない、保守性の低いコードにしてしまったことが多かった。最初に書いた形は、その時点の頭の中の思考の流れにより近く、読み手もおそらくそのように解釈する可能性が高いという利点がある。過度なリファクタリングをすると、それが失われることがある
結局、プログラミングは工芸に近いので、状況に合った選択は経験が助けてくれる
目的はただ一つだった。アプリの片隅で、あるプラットフォーム向けに使っていた個別のHTMLページを、別のプラットフォームのネイティブ感をまねたカルーセルに変換する作業で、そのプラットフォームとアプリのその領域に極度に特化していた
9つのスコープをそれぞれ関数にすることもできただろうが、そうすると開発者たちは再利用したくなったはずだ。各段階には、前の段階で起きたことに関する微妙な前提があり、別関数にするなら、その前提を見直し、一般化し、各メソッドが独立して動作するかを検証する必要があった。ほかの場所ではほとんど必要ないコードに、そんなコストをかける理由はなかった
デバッグが難しくなることもなく、エンドツーエンドテストもあり、中間段階の状態が関数の外に漏れることもなかった。実際、別の開発者2人が時間をかけて修正に参加し、うまく動作し、作成も速かった
線形コードは十分にスケールし、問題を解決する。常に望ましい形ではないが、思った以上に多くの場面で作業をずっと楽にしてくれる
最初に2000行の怪物を見たときの反応は良くなかったが、5分眺めれば実際の欠陥は見つけにくく、テストがいくつかあれば、現実化しない恐れだけだった
ある時点で、その数十個の関数が特定の順序で呼ばれなければならず、それぞれ一度しか使われていないことに気づく。結局、誰かがそれらの関数を有用に使うには、魔法のような組み合わせ順序を知っていなければならないよう強制しているようなものだ
巨大な線形関数がしばしばより読みやすく望ましい核心的な理由は、複数の概念と関係をコンテキストスイッチなしに一つの塊として同時に保持でき、理解を助けるからだ。極端な支持者としてK言語の発明者Arthur Whitneyがいて、1画面にできるだけ多く収めるため、非常に簡潔で他人にはほとんど理解不能なコードを書く
個人的な例では、大きな
switch文にビジネスロジックが入っている巨大なWindowsメッセージ処理関数、つまりWndProcを読んで理解しデバッグするほうが、メッセージハンドラを別関数に分割したVisual C++版よりはるかに簡単だったまた、マイクロコントローラのサンプルコードで、ADCの使用例が1ファイルにすべて入っている版と、
main.c、config.c、interrupts.c、timer.cなど複数ファイルに分かれた版があったが、200行にも満たないのに後者はコンテキストスイッチのせいで理解しにくかったこうしたコード片はたいていクラスの
private関数になり、状態を持つ。private関数なので実際にはテストもしにくいこうして、一度しか呼ばれず、たいてい副作用で状態を変更する
private関数が大量に生まれる。呼び出し元のすぐそばにあれば単純な場合にはまだ読めるが、時間がたつと誰かが呼び出し関数と切り出した関数の間に別の関数を追加するすると、呼び出しグラフを見るかクラスファイル内を検索しない限り、どこから呼ばれているのか分からないコード片が、互いに異なる副作用の状態を変更することになる
コードを非線形にするなら、言語が対応している場合、切り出した
private関数を呼び出し関数の内部関数にすることを少なくとも検討してほしい。そうすれば、ほかの場所から呼ばれないことが明確になる実際のコードベースでは、これも二者択一ではなく、両者を組み合わせて読みやすく保守可能な形にする、芸術に近いものだ
人々はそのすべての分岐をテストするだろうか。それともピザを一つ入れるテストだけを書いて、だいたい動くかを見るだけだろうか。外側から複数の分岐をテストするのはたいてい面倒で、小さく特化した関数をテストするより厄介なので、後者である可能性が高そうだ
「線形コードはスケールしない」という言い方は、むしろ逆。大きなコードベースで本当に悪夢になるのは、深くネストしたコールスタックを持つ小さく簡潔な関数群
新しいコードをどこに追加すべきかが明確でなく、そのコードが呼ばれ得るすべての経路を追跡しなければならないため、変更の影響範囲を把握する難度が指数関数的に上がり、重複したサブルーチンも生まれる
99%の場合、良い抽象化を作れているわけではないので、素直に線形コードを書くほうがいい。疑わしい関数セマンティクスよりコピー＆ペーストを好む
print_table()を追加すると、誰かがそれを見つけて自分のコードで使い、自分のユースケースに合わせて出力を調整する小さなフラグを付けること12か月後にはこうなる:
print_table(rows,headers = None,is_unicode = False,left_align = False,align = [],remove_emoji = None,max_width = 80,potato_mode = 7,_debug_frontend = not FLAGS.dont_debug,ellipsis_for = 0,no_print = False,)可読性がスケーラビリティに影響し得るという事実を除いて、2つの概念を直交するものとして見るなら、線形コードはモジュール型コードほどにはスケールしない。この二分法は知っておく価値があり、状況に応じて考慮する価値もある
それでもなお同意しない。小さな関数が純粋関数なら、可読性の問題は起こさない。状態を触らないという意味であり、コードにロジックを注入せず、依存性注入や関数を別の関数へ渡すことを明示的に最小化すべきだ
データだけを渡す純粋関数のパイプラインを作れば、読みやすく、スケール可能になる。設計上の欠陥のせいでロジックを書き直さなければならないケースははるかに減り、純粋関数を組み合わせるとコードはレゴのようになる。リファクタリングも、既存のプリミティブ要素を再構成し、組み替える作業に近くなる
サンプルコードは、少なくともピザの比喩を意味のある形で保とうとしていたか、低水準なGoコードでなければ、もう少し気が散らなかったはず
prepareは関数名としてひどい。熟練したGopherならNewPizzaFromOrderのような名前を付けたと思うaddToppingsを別関数にする理由が見当たらない。どうしても必要なら、個人的にはfunc (p *Pizza) WithToppings(topping ...Topping) *Pizza { /* ... */ }のようなPizzaのメソッドにしただろう。実際のピザは可変なので、メソッドがレシーバを変更するピザを焼くたびに新しいオーブンをインスタンス化する理由も分からない。既存のオーブンから始めて
oven.Preheat()を行い、oven.Bake(pizza)を呼ぶべきだ。さらに進めて、oven.Preheat()が.Bake()を公開するOvenの新しい型を返すようにし、予熱なしで焼いてしまうミスをコンパイル時に防ぐこともできる。別の場所にはBakerインターフェースがあり、予熱がそれほど重要ではなく不要なToasterOven実装もあるかもしれないコードを変えないとしても、宣言順は予測しやすい流れに合わせて並べ替えたはずだ。そうすれば、互いに呼び合う関数をざっと読むときにページを上下に飛び回らずに済む
時間がないのでここでやめるが、このコードは「どちらが読みやすいか」という議論を始めるには、すでにあまりにも悪い例だ
John Carmackもほぼ同じことを言っていて、それ以来ずっと従っている。線形コードは実行順に沿っているので当然読みやすく、視線のジャンプを最小化する
コードの中には再利用のために非線形であるべきものもあり、その場合、実行はグラフになる。コードがグラフ構造の再利用を活用していないなら、1本の辺で足りる場所に頂点を導入する必要はない
http://number-none.com/blow/blog/programming/2014/09/26/carm...
この場合、左側のコードは
pizza.Toppings = get_pizza_toppings(order.kind)のようにしていれば、メイン関数ではピザの変更が中心に残って、より良かったと思う線形コードのほうが読みやすいという点にはある程度同意するが、それだけで良いコードプラクティスになるわけではない。
良い線形コードはより読みやすいと思うが、保守性とテスト容易性ははるかに落ちる。数十年の経験があり、CS 学生の外部審査もしているが、何年も現場で見てきた良いプラクティスの中で確実に言えるのは、関数を小さく保つことだけだった。
抽象化が特に好きなわけでもなく、コード重複を何としても避けるべきだとも思わないが、できるだけ単一目的に近い関数を作っておくと、未来の自分に感謝される。
例のようなコードが 10 年間プロダクションで動くと、各区間は変わっていく。運が良ければコメントも更新されるだろうが、たいていはそうならない。単体テストも大きく扱いにくくなり、次第に雑になっていき、誰かが変更と明確につながっているように見えないテスト部分の修正を忘れるかもしれない。コードも時間とともに読みづらくなる可能性が高い。意図や無能さのせいではなく、時間的プレッシャーのような人間的な理由によるものだ。
完璧な世界なら関心事を分ける必要はないだろうが、私たちは不完全な世界に生きており、関数が小さく責務が少ないほど、時間が経ってもその不完全さを扱いやすくなる。
オブジェクトを特定の状態の流れに通しているなら、それを分割して型で遷移を示すか、そうでなければ 1 つの大きな関数として書くほうがよいと思う。たとえば
bakePizzaがRawPizzaを受け取りBakedPizzaを返すなら、呼び出し順をコンパイル時に強制できる。読みやすさ、正確性、テスト容易性のために前者を好むが、ほとんどのプログラミング言語ではオブジェクトの型を変えるには新しいオブジェクトを作る必要があり、実行時コストがかかる。ホットなコードパスならインプレース変更は妥当で、その場合は 1 つの線形関数に置くほうがよい。
https://mitpress.mit.edu/9780262045490/
John Carmack の関連メール: http://number-none.com/blow/blog/programming/2014/09/26/carm...
議論: https://news.ycombinator.com/item?id=12120752
強く同意する。以前は反対陣営にいた。
ここでの根本的な緊張関係は、一方の振る舞いの局所性と、もう一方の高レベルな「目次」ビューを明確に示したいという欲求の間にある。読みやすいコードには局所性のほうが重要だ。記事で述べられているように、目次的な観点はセクションコメントで十分明確にできる。
線形コードを好むべき、より重要な理由もある。コードベース全体を探索するとき、「塊」、つまり関数やクラスや言語が強制する単位が、おおむねビジネスのユースケースに対応していると、はるかに楽になる。そうでないと探索空間が大きくなりすぎ、断片から全体を自分で再構成しなければならない。コード構造がその仕事を代わりにしてくれるべきだ。
複数の「もの」がすべて 1 つの仕事、たとえば登録や購入に関係しているなら、コード上でも 1 つにしておくほうがよい。探すのも変更するのもずっと簡単だ。再利用が必要になったときだけ下位関数に分け、整理のためだけに分けるべきではない。
[0] https://htmx.org/essays/locality-of-behaviour/
最大の理由は状態だ。関数が長いほどローカル変数のスコープが広くなる。関数のどこからでもどの変数でも変更でき、データフローがすぐには明確でない。関数が多いとスコープは小さく保たれ、データフローがより明示的になる。
副作用としてインデントも減る。
同時に、小さすぎる関数は好きではない。実際の処理がどこで起きているのか見つけにくくなるからだ。
順番に実行しなければならない、再利用不可能な 10 個のステップがあり、各ステップが 100 行ある日次締め処理を考えてみよう。各ステップは前のステップと似ているが同じではないデータを使う。本当に 1000 行の単一関数を選ぶのか？
どちらも線形に読める。小さな関数を切り出したバージョンはページ上部に目次があり、ステップ間のデータフローを要約している。全体を読むつもりなら、魅力的な読み順に見える。
ただしこの可読性を保つには、ステップの順序が変わったときに関数の位置も移動しなければならない。
private関数で、目次からしか呼ばれないなら問題ない。だが何も順序を保つよう強制しておらず、全体の読みの流れを意識するよう強制しているわけでもない。関数が再利用され始めると、もはや線形化できない場合がよく起こる。ときには人々が諦めてアルファベット順に並べたり、単にランダムになったりする。
経験上、ある人がコードに慣れているほど、コードを小さな関数に押し込めるのが正しい道だと考えがちです。
その人はすでにそのコードのメンタルモデルを築いているため、その人にとって最もきれいな実装は行数が非常に少ない実装です。
しかし次の人が来ると、元の文脈なしに同じメンタルモデルを作るため、あちこち行き来しながら頭の中のスタックを push/pop しなければならず、これはずっと難しくなります。
たとえば、使っている言語の標準ライブラリのソースコードをどれくらい頻繁に読みますか？ ほとんど読まず、通常はメソッドシグネチャを見て、少し複雑だったり新しかったりすればドキュメントを読みます。
インターフェースの核心は、メソッドがどう実装されているかではなく、何をするのかだけを気にさせることです。それは文脈、名前、ドキュメントの組み合わせで説明されます。しかし多くの開発者はこれを理解していないか気にしていないため、線形であれモジュール型であれ、筋の通らないコードを書きます。
たとえばサービスクラスで、あるメソッドを呼んで何らかのデータを取得し、別のメソッドで別のデータを取得し、3つ目のメソッドで前の2つと組み合わせるべきデータを取得しなければならないとしたら、そのサービスの意味は何でしょうか？ 内部の複雑さをすべて外にさらしていることになります。
小さなメソッドを強制しようという話ではありません。一度しか呼ばれず、非常に具体的なことをし、正しい順序で呼ばれなければならない5行の関数が20個あっても意味はありません。それはクリーンコードではなく、カーゴカルトプログラミングに近いものです。
重要なのは、新しいチームメンバーにも熟練メンバーにも筋が通り、推論しやすく、複雑さが適切な場所に隠れるように、適切に抽象化することです。簡単ではありませんが可能です。
私の息子は十分賢かったにもかかわらず学校で苦労しており、複数の専門家のうちの一人が、学校は概してボトムアップで教えるが、息子は非常にトップダウン型の学習者だと説明しました。彼は詳細に入る前にまず概要が必要で、他の人たちは詳細を先に把握してから概要を組み立てる必要があります。学校は通常、後者のグループに合わせて教えます。
プログラマーの間にも似たような違いがあるかもしれません。
前の開発者が
BakePizza関数を書いたなら、ピザは正しく焼けると仮定して次の行に進めばよいのです。レストランの運営方法を理解しようとしながらオーブン温度のような細部に入り込むと、レストランがどう動いているのかも理解できず、正確なオーブン温度も忘れてしまいます。エディタには関数を一時的にインライン展開するトグルがあるべきです。もう行ったり来たりする必要はありません。