Ifは上に、Forは下に プログラミング効率化の考え方

確かにね。でも、文書化されたインターフェース契約に対してプログラミングすることもまだできるよ。コンパイラは型以外の部分の違反は捕まえないけど、ちゃんとしたインターフェース契約があれば要件はしっかり定義される。トレードオフだけど、プログラムの色んな場所で同じ条件分岐を繰り返したり、その判断に必要なコンテキストをあちこちで引き回したりするのも同じようにトレードオフなんだ。

少なくともコンストラクタに押し込めることはできるよ。

＞見事な視野狭窄、頑固さ、そして最大限に一般的な代数的および任意の制約型システムを目指す使命感で
それはkeyhole optimizations、greedy search、そして”the customer is always right…”って呼ばれてると思うな

考え方とか例は型システムが面倒を見てくれるってことだよね。その経験則はちょっと一般化しすぎてるかも。null許容じゃない型がある場合のnullチェックみたいなことにもっと近い話だと思うな。

俺はそう思わないな。だって、もし早い段階で仮定を置けば、その仮定がプログラム全体に適用されて、考えるのが楽になるじゃん。

入力データを整形して正規化したら、コアロジックでの仮定ははっきりするはず。それは正規化されたデータのルール次第だよ。全部の呼び出しチェーンを知る必要はないんだ、なぜなら”narrow waist”を作れば、そこで全てが明確になり、エラー処理もされてるから。だから、入り口から”narrow waist”までと、”narrow waist”から終わりまでを知ってればいいのさ。

「関数内のifは呼び出し元に移せないか考えろ」って考え、反例多すぎだろ。例えば37箇所から呼ばれてたら、全部にif書くの？
getaddrinfoとかEnterCriticalSectionみたいなAPI関数ならどうよ？
この考えは、せいぜい2箇所から呼ばれる内部関数で、かつ決定がその関数の関心外にある場合だけ適用できると思う。
ループからifを出すのは最適化のためだけど、最適化は良い設計とぶつかることが多いんだ。呼び出し元が条件を知るのは設計として良くないかもね。これはOOPのメソッド呼び出しでも同じ問題があるよ。ループからメソッド呼び出し（ifに相当）を出すテクニックは、設計思想に反することもあるんだ。

「関数が37箇所から呼ばれてたら、全部にifを繰り返すのか？」って質問だけど、多分そういう場合はtrueとfalseの処理を分けて2つの関数にして、それぞれ21箇所と16箇所から呼べばいいんじゃない？

それは条件がconstant-foldableな場合だけ可能だよ。
ifの部分をinline functionにすれば実現できる。
前：function(cond, arg){ if (cond) { true logic } else { false logic } }
後：inline function(cond, arg) { cond ? function_true(arg) : function_false(arg) }
こうすれば37箇所をいじる必要なし。関数はインライン化されて、condが定数なら条件分岐は消えるよ。

ここでのキーワードは「consider」（検討する）だよ。この記事はtagged unionsとかを使う場合の、ある程度特定の設計問題を対象にしてるんだ。

37回呼ばれるならリファクタリング検討だけど、場合による。ライブラリ関数（EnterCriticalSectionとか）はドメイン境界だから例外だね。アプリ内部ならifは安全に外に出せる。ライブラリでも端っこには置ける。自分のドメイン内で制御フローを端に寄せるのが良い。
ただ、これはidiomであり、文脈に応じて賢い判断が必要だよ。

N回以上呼ばれてるからってリファクタリングするのは、関数によると思うよ。前の人が言ってたように、ロック関数とかmemcpyは多くのプログラムでいっぱい呼ばれるだろうしね。実際、うまく分割された機能って、色んな場所からたくさん呼ばれることが多いんじゃないかな。

37回以上呼ばれるくらい役立つ関数が一つもないような大きなプログラムなんて想像できないな。
メモリ確保？出力？リストに要素追加？ファイル書き込み？
多分他の意味で言ってるんだろうけど、役立つ関数は同じプログラム内で何度も呼ばれちゃいけない、とか思ってるわけじゃないよね？

ねえ、同じ関数を何箇所から呼ぶのが適切なの？5箇所は多すぎる？6箇所なら？7箇所になったら全部リファクタリングしなきゃいけないんだよね？まさか。

この話、使う前に毎回チェックが必要な場合にしか使えないよ。そういう関数って、そもそも良くないコードだけど、呼ぶ場所が少ないならマシ。その場合は呼ぶ側でチェックが正解。もしその関数を使いまくるなら、コードが重複しまくって、自分のやり方が変だって気づくかもね。

じゃあさ、関数がポインタ使うとき、中でnullチェックするのはダメってこと？パフォーマンスの問題なきゃ、単に好みの問題だろ。こういう「だいたい合ってるけど、ちょっと違う」ってのがコードをメチャクチャにするんだ。「Ifは上に、Forは下にしろ！」とか、完全に嘘くさいルールで、設計に変な制限つけてるだけじゃん。

Ifを呼ぶ側に移すかどうかでパフォーマンスに違いが出るなら、inline関数を使えばいいよ。コンパイラが賢い場合、ループ内でポインタが変わらないことを見抜いてifをループの外に出したり、ポインタがnullにならないと分かればifを完全に消したりしてくれることもあるから。

俺的には、nullじゃないってassertすべきだと思う。関数にnullポインタ渡す状況なんて、絶対ありえないだろ。

ISO Cだと、free(NULL)とかfflush(NULL)とかtime(NULL)とかstrtol(text, NULL, 10)とかsetbuf(stream, NULL)とかrealloc(NULL, size)みたいに、NULLを渡すのがOKで、意図された動作をする場合があるんだ。POSIXとか他のAPIにもそういう例はいっぱい。現実が君の意見を打ち砕いたな、残念でした！

ごめん。そういう関数作った人たちが、良い選び方をしたって言ってるの？彼ら、一つの入力値で関数の動きを結構変えちゃったんだよ。その値って、バグの可能性の方が高いのにさ。

そうだよ。多分、これからも似たような選び方をするだろうね。

はっきりしたルールなんていらないよ。ただ、平均的な「頑張りすぎでちょっとズレてるHN投稿者」より賢くなって、同じような呼び出し方をいつやりすぎてるか気づけばいいだけ。

「関数が37箇所から呼ばれてるなら、コードをリファクタリングする必要がある」
マジ？
反論はすぐ思いつくよ：認証。全部のAPIからauthenticate()を何らかの形で呼ぶでしょ？その37個全部から。

もしそれぞれのAPIで明示的にauthenticate()を呼んでるなら、それは「間違ってる」やり方だよ。そういう場合は、明示的な認証じゃなくて、暗黙的な認証が必要なんだ。全部のAPI呼び出しで呼ばれるようなミドルウェアに移せばいいじゃん？

だって、authenticate()じゃなくてmiddleware_caching_auth_broker()を37箇所から呼ぶことになるだけじゃん。名前が変わっただけで、37箇所は変わらないよ。

いや、Middlewareってそういう仕組みじゃないよ。フレームワークに登録するんだ。そうすれば、リクエストがエンドポイントに来る前に、フレームワークが勝手に呼んでくれるんだ。だから、全部のAPIコールで認証が呼ばれたって信頼できるんだよ。

でもさ、呼び出しがプログラマーから隠されてるなら、それでいいじゃん。この返信、真面目なのか皮肉なのか自分でもわかんないけどね。たぶん両方かな。

＞middleware_caching_auth_broker()を37箇所から呼ぶことになる…＜
いや、違うよ。実際にはどこからも呼んでないんだ。君が書いてないフレームワークが、登録されたmiddlewareを呼んでるんだよ。この記事で問題にしてるのは、37箇所から呼ばれることによるコード構造の複雑さだろ。登録されたmiddlewareは、そういう”コード構造の複雑さ”が問題になるような場所からは呼ばれないんだよ。君の理屈は、『プログラミング言語でコード書いたからビットシフトを何百万回も呼んでる』って言うのと一緒だよ。技術的にはそうかもしれないけど、ここで話してることとは違うんだ。

それって、食料品はスーパーからしか来ないと思ってるときのプログラミング版みたいだね。

それはいいけど、農家が存在しないとか、システムの一部じゃないなんてフリはしないでね。

あの発言の最も妥当な解釈は、『関数が37回呼ばれてるからリファクタリングが必要だ』（それはむしろ良いことなのに）ってことじゃなくて、if文をその関数の内外に移動させようと思ったときに、リファクタリングが必要になるってことだと思うよ。

記事の”dissolving enum refactor”の例はポリモーフィズムだよ。最初にifで場合分けし、それをオブジェクト等に持たせて、メソッド呼び出しで処理する。こうすると、場合分けを決める場所と処理する場所が分離できる。ケースが変わっても、決める場所だけ直せばOK。これはトレードオフで、処理箇所でケースが見えなくなる代わりに、ケースリストへの依存を減らせるんだ。

コードの複雑さチェックツール（SonarQubeとかね）は、最終的にifを下に押し下げる方向に促すんだ。でも、この記事はその逆を勧めてるよね。ifを上に押し上げると、複雑な分岐ロジックが一個の関数に集約されるけど、実際の作業は全部、単純なサブルーチンに任せられるって考え方。

これを解決する方法は、決定と実行を分けることだ。これは昔なじみのBertrand Meyerから学んだ考え方だよ。
if (weShouldDoThis()) {
doThis();
}
これは関数型コア・命令型シェルを補完するか、その一部だね。これらのチェックを分離することでテストしやすくなるし、複雑さが気になるならチェック内の各条件を関数に分けられるよ。

関数は決めるか実行するか、どっちかだけにすべき。両方やっちゃダメだね。

でも、もしそれが全部なら、システムはどうやって動くの？結局どこかで決めて、その決定に基づいて行動する必要があるでしょ？

一つの可能性としては、フレームワークから呼ばれるfile.pyみたいなファイルを使うことかな。インターフェースはこんな感じ。
def doth_match(*args)：
return True # 条件

def doeth_thou(*args)：
# 処理
return {} # 例：ステータスオブジェクト

フレームワークがループして最初の関数をチェック、真なら二番目を実行。他のルールファイル（やオブジェクト）についてもブレークしたり続けたりできる。rule1.pyとか複数ファイルで分けてもいいね。

親コメントが言いたいのは、フレームワークのどこかでdoth_matchを呼んでからdoeth_thouを呼ぶ場所では、決定と実行の両方をやってる一つの関数があるってことだと思うんだ。プログラムには両方やる責任を持つ関数が絶対必要になるはずだよ。

質問して、その答えで別関数を呼ぶような関数は、ただの糊付けコードで、大したことないってば。開発者が絶対的なものを求めるのは卒業すべきだって。principle of the excluded middleみたいな考え方は、ブール論理とか特定の分野以外ではあんま使えない、一発芸みたいなもんなんだよ。

これはただ、関数の中の決定を呼び出し側に移動させるだけだね。そうなると、関数自体は一つなのに、呼び出される場所がたくさんあるから、間違えやすくなることが増えるよ。

これに加えて、ここでめちゃくちゃ役に立つパターンがあるよ：
findThingWeShouldDoThisToは条件を満たすものを見つけられるし、もし対象となるものを渡せればdoThisをすごくシンプルにできるんだ。それは読み取り専用で、テスト可能だし、読みやすい。超おすすめだよ。

なんか分かりづらいな。本来は条件とアクションを分ける話でしょ？あなたの例だと「これをすべきか」のロジックがどれくらい関数にあるか不明だよ。もし全くないならfind/lookup関数でOK。でも、もしセットを回したりDBをクエリしたりするロジックがあるなら、元の話と違う。
たぶん、doThis( findAllMatchingThings( determineCriteriaForThingsToDoThisTo()))
みたいにするのが良い関心の分離じゃないかな。

コードスキャナーの報告は鵜呑みにしちゃダメ．Sonarとかもバグじゃない”コードスメル”を出すし，それを直すのは時間かかるし，逆に新しいエラーを生むこともあるからね．実際のヤバい問題に集中できなくなるよ．

同意．サイクロマチック複雑度チェックは個人的に一番苦手なルールかも．ベテラン開発者ならツールより複雑さの良し悪しは分かるはず．でも，100行で1000個のことやる関数作るような開発者には，CCCがリファクタを強制する偶然の助けになるのは認めるしかないな．

サイクロマチック複雑度はデカい関数見つけるのに役立つけど，制限値を低く設定したがる人は，ループの中にifがあるくらいのレベルでも文句言って，結果何もしてない極小関数だらけのコードになるのが問題．どこで何やってるか追いにくいから，良い設計じゃないよ．

これを俺は”poltergeist code”（ポルターガイストコード）って呼んでるよ．小さい関数がいっぱいあって，全体では複雑なこと正しくやってるんだけど，どこでどうやってるか追うのがめちゃ大変なんだ．

こういう時の反論ルールに「使い回さない関数は分けなくていい」ってのがある．絶対じゃないけど．理解しやすくなるなら分けるのもあり．どっちか迷ったら全体の意図が明確になる方を選べ．ボスや顧客のメールに近いのはどっち？「3番目の単語にスペースがあるか見て…」なんて普通書かないでしょ？
判断に迷ったら「この関数何のため？」って聞け．細かい操作に名前つけたい理由は何？

分かる．特に若手には良いチェックだと思うよ．俺の経験上，サイクロマチック複雑度の問題ってネストした条件分岐として出ることが多いんだ．自分でも最小限に抑えてるけど，深いネストしてる同僚もいて気になる．早めにリターンするか，ロジック分けてネストをせいぜい2レベルにしたい派．ベテランなら判断できるべきだけど，基準あってPR戻さずに済むならそれはそれでいいと思うよ．

こういうルールって，AIが書いた適当なコードっていうか”雰囲気コード”を見つけるのに役立つかな？レビューで指摘する手間を省けるかもって思うんだよね．

こういうツールって，だいたい何かしらのコンプライアンスのために必要だったりするんだよ．デフォルトルールいじるのは，コードフォーマッターみたいに全部無駄になっちゃうかも．出荷時のデフォルトを変えるように提唱するのが一番だけど，”そんな暇ねぇよ”って感じだよね™．