goto は悪だ、にも通じるところもあるのだけれど。
一つ前何個か前で「わかりやすい主張には皆熱中しやすい」と書き、そこで「マジックナンバーへの過度な反応」とさらっと書いたので、そのことについても書いておこうと思う。
「マジックナンバーは悪だ」の主張は、本来のものは非常にわかりやすい:
1 return 31536000 + is_leap * 86400;
これが「31536000 てなんだよ、86400 てなんだよ、わかるように書けやこのばかちんが」というのが、それこそ「みんな大好き爺カーニハン大明神さまさま」だとかロブ・パイクだとか、とにかく「色んな教育熱心な先達」によって、口うるさく言われてきた、「伝統芸能」である。無論このことに異論などあるはずがなく、「せんせー、大好きっ」である。
そして「だからこそ」、皆「絶対視し、根絶に夢中になる」。ほかのもっと根幹にかかわる大事な大事なことを「ほっぽらかしてでも」。
それこそワタシが共感しないと言ったばかりの JSHint のポリシーに関係するのだが、まず「Correctness (正しさ)」の話なのか「Maintainability (保守性)」の話なのかをきちんと区別することが必要である。
「保守性が欠けるプログラムは正しさも危うい」というふうにこれらを一体と考えるのが、いわばスタイルチェッカの責務なわけで、そのことで JSHint を非難したわけだが、だからといって「正しさ」と「保守性」の優先度が「ない」と考えるのもこれは違う。そして「マジックナンバーの件」は「保守性の問題である」と考えることが出来ないエンジニアがなぜか多いのも気になる。
問題の話はここから。つまり、「根絶には夢中になる」割には、その本来の「保守性の問題」に全然意識的になれないエンジニアが非常に多いのだ。まず、「誰がいつどのようなタイミングで「保守」を行うのか」の想像力を欠いたまま、「定数は一箇所にかき集めれば便利」ルールに従って、一枚岩の「バカでかい定数かき集めファイル」を発明する。無論このアプローチにも正当な理由がないことはないが、「保守性」を考えた結果そうするのではなく、「それしか思いつかない」からそうするわけである。だいたいにして、その種の「マジックナンバー」は、「全体で共有されるべき知識」である場合もあれば、そうでない場合も多い。つまり、「特定の処理にだけ秘匿されるべきマジックナンバー」はあるのである、多いのである。
非常にわかりやすい例。まず、「日付時刻を扱うユーティリティ」というものを書きたい、とする。なおかつ、「アプリケーションの振る舞いを変えるためのプロパティのデフォルト」を管理したいとする。しかるに「定数は一箇所にまとめなければならない」に従うならばこうなる:
1 public class ApplicationConst {
2 private void ApplicationConst() {}
3
4 // --- 日付時刻関連 BEGIN
5 public static final int TOTAL_SECONDS_OF_YEAR = 31536000;
6 public static final int TOTAL_SEDONDS_OF_DAY = 86400;
7 // ... 以下大量に…
8
9 // --- 日付時刻関連 END
10
11 // ...ほかにもゴロゴロ大量に
12
13 // --- アプリケーション設定デフォルト関連 BEGIN
14 public static final Point DEFAULT_START_POSITION = ...;
15 public static final Point DEFAULT_WINDOW_WIDTH = ...;
16 public static final Point DEFAULT_WINDOW_HEIGHT = ...;
17 // ... 以下大量に…
18
19 // --- アプリケーション設定デフォルト関連 END
20
21 // ...ほかにもわんさか様々に
22
23 // --- 共通 BEGIN
24 // 以下使用は非推奨だが、どうしても名前が付けられない場合に限り、checkstyle を静かにするのに
25 // 利用してよい。(本質が埋もれるよりはマシ。)
26 public static final int ONE = 1;
27 public static final int TWO = 2;
28 public static final int THREE = 3;
29 // ... 以下「膨大に」…
30
31 // --- 共通 END
32 };
こうした「管理」が「保守性」が良いか悪いかなんてことは「少し考えればすぐにわかる」ことなのに、こういうことをするエンジニアは自分の正義を疑わないので、たとえ VCS でエンジニア間の更新のコンフリクトが「毎日」発生しようが、一向に気にしない。
今の例は「保守性」の話でもあるが、「一箇所に集める」という発想そのものが「カプセル化のポリシーに反する」ことが「なぜだか忘れ去られる」ということも表している。すなわち、今の場合「日付時刻関連」は「日付時刻関連の処理」に閉じ込めてしまうべき知識なのである。おそらく「一箇所触るだけで全体が従ってくれる」というメリットをどこかで中途半端に聞きかじるからこれ「だけが正義」になるのだろうが、なぜそれだけで「みんな大好きオブジェクト指向」の根幹を揺るがすような破壊をしていることに気づかないのかは、ほんと理解に苦しむ。(言っちゃ悪いがそういうのに限って「おぶじぇくとしこうまんせー、java まんせー」タイプである。)
そしてこれも以前書いたことがあるが、「最終的にその「定数」を誰がどのように管理するのか」(保守)について意識的でなければ元来おかしいわけである。多くは「保守担当エンジニア」だろうが、「顧客」の場合もある。が、「顧客」に「java ソースを触らせて、コンパイルもさせる」という「設計」をする「わけがない」のであって、そうしたものは「設定ファイル」に書き出しておくのが普通である。そしてかなりのバカエンジニアが「円周率」を設定ファイルに書き込むのだ。それはあんたの持ち物ではない。
もう一つ。「定数だ」⇒「const にして定数ファイルにかき集めよ」。ヲィ。「円周率」を例にしたのはわけがある。つまり、「既にあるのに」発明してしまうバカもこれは「とてつもなく多い」。そしてそれが保守性を悪くしていることにも気付かずに、「定数ファイルに追い出すオレ、ちゃんとしてるぜっ」と譲らない。
ワタシが「過度の反応」と呼ぶのはこの構造である。「マジックナンバーは悪だ」は「保守性を悪くするからだ」という「意味」を理解しないまま闇雲に絶対視するからこんなことになる。そして「悪は悪だがもっと大事なことがある」ことを忘れてしまうのは、ひとえに「あまりにもわかりやすく、昔から言われ過ぎているから」なのかもしれない。
2021-04-21「3年後の上塗り」:
3年経とうが他愛のないままの話として。
たまーにこのページにぱらぱらとアクセスがあるようなので、ワタシもつられて読んだりもしてるので、なんとなくこのページはずっとワタシの側にいる気がしてるんだよね。で、ふとこのネタに関係しそうなことを(python で)「やらかしてみた」:
1 import ctypes
2
3 # ...
4 def _is_ntfs_compress_target(fn):
5 f = ctypes.windll.kernel32.GetFileAttributesW
6 return f(fn) & 0x800
「0x800って何やねん、万死!」てことに、なる? ならない?
実はこのパターン、「異種言語交流」で頻出で起こる悩みだったりするの。なので、ちょっと一言いってもバチは当たらんかなと思って。きっといつかあなたもこれについて思い悩む日が来る、かもしれないし、来ないかもしれない。
今例にしたもので言えば、「FILE_ATTRIBUTE_TEMPORARY」などの定義はこれは、「C 言語のためのヘッダファイル」に定義されている。実現方式にはバリエーションがあるかもしれないが、典型的にはたとえば:
1 #ifndef FILE_ATTRIBUTE_COMPRESSED
2 #define FILE_ATTRIBUTE_COMPRESSED 0x800
3 #endif
みたいな感じで、これを取り込めるヘッダファイルを include することにより、「0x800 ってなんだよ、ちゃんと FILE_ATTRIBUTE_COMPRESSED て書けやヲラ」という正義を全うすることが出来る。「C/C++言語なら」。このシンボル定義の恩恵を python 利用者が直接受けられるかといえば、これは「先達次第」。先に苦労してくれた誰かが「マジックナンバーは悪だ」と言う正義を発揮して、どこかのモジュールに定数表を作ってくれているなら、使える。使える、けれどもそれは、純然たる「重複定義」に他ならないし、もっと言うなら、「python においては整数とて Python オブジェクト」なので、python における以下:
1 FILE_ATTRIBUTE_COMPRESSED = 0x800
は、上の C でのものと全く意味が違う。「性能」の話をするなら、何倍もランタイムコストのかかる演算である。
性能、の問題はともかくとしても、この「重複定義の保守コスト」というものが馬鹿にならない、と思うかどうか、の話ね。ワタシが「やらかした」部分のコードというのは何をしているかと言うと、NTFS でフォーマットされたディスクに置かれたファイルに対して、「内容を圧縮してディスク領域を節約する(C)」にチェックされているかどうかだけを問い合わせている。ほかには何も知る必要がなく、ほんとにそれだけを知りたい、というコード。このために、果たして以下を保守するか?
1 FILE_ATTRIBUTE_ARCHIVE = 0x20
2 FILE_ATTRIBUTE_COMPRESSED = 0x800
3 FILE_ATTRIBUTE_DEVICE = 0x40
4 FILE_ATTRIBUTE_DIRECTORY = 0x10
5 FILE_ATTRIBUTE_ENCRYPTED = 0x4000
6 FILE_ATTRIBUTE_HIDDEN = 0x2
7 FILE_ATTRIBUTE_INTEGRITY_STREAM = 0x8000
8 FILE_ATTRIBUTE_NORMAL = 0x80
9 FILE_ATTRIBUTE_NOT_CONTENT_INDEXED = 0x2000
10 FILE_ATTRIBUTE_NO_SCRUB_DATA = 0x20000
11 FILE_ATTRIBUTE_OFFLINE = 0x1000
12 FILE_ATTRIBUTE_READONLY = 0x1
13 FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS = 0x400000
14 FILE_ATTRIBUTE_RECALL_ON_OPEN = 0x40000
15 FILE_ATTRIBUTE_REPARSE_POINT = 0x400
16 FILE_ATTRIBUTE_SPARSE_FILE = 0x200
17 FILE_ATTRIBUTE_SYSTEM = 0x4
18 FILE_ATTRIBUTE_TEMPORARY = 0x100
問題が「0x800 ってなんだよ、マジックナンバーは悪だ」なのだとして、だよ。確かに理解し難いコードになる可能性はあるかもしれないが、今問題にしているコードに関しては、python なのだから docstring の説明が充実していれば済むのかもしれないし、関数名の工夫でもいいのかもしれない、と考えて良い? 悪い? こうなってたらどう思う?:
1 import ctypes
2
3 def _is_ntfs_compress_target(fn):
4 """
5 「内容を圧縮してディスク領域を節約する(C)」にチェックされているかどうか。
6
7 GetFileAttributes を呼び出して、FILE_ATTRIBUTE_COMPRESSED (0x800)
8 ビットが立っているかどうかを調べる。
9
10 see: https://docs.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants
11 """
12 f = ctypes.windll.kernel32.GetFileAttributesW
13 return f(fn) & 0x800
それでもなお「0x800 ってなんだよ」と思うのかどうか。どう考えようと、基本あなたの自由だ。ワタシ個人としてはこれは「バランス感覚の問題」だと思っているので、都度どうするのがいいか考えればいいと考える。
たった一つの言語だけ使って仕事してる、みたいに、エンジニアとしての経験がまだ浅いうちはなかなかわからないかもしれないけれど、多数の言語を駆使し、頻繁にいろんな言語を行き来するようになってくれば、このパターンの悩みが尽きないことは、だんだんわかってくると思う。そういうこともね、少しずつ知っていったらいいと思うよ。
2022-06-30「さらなる上塗り」:
とあるビデオをみてて、ワタシが良くしてる主張に名前がついてることを今さら知った。これ:
英語のビデオからここに誘導されたので英語のを貼り付けたけど、日本語ページも出来てるよ。
「ESLint: no-magic-numbers についての「くだらない話」」には関係あるともないとも言える、かね。「Cargo cult programming can also refer to the practice of applying a design pattern or coding style blindly without understanding the reasons behind that design principle.」部分は対応してるでしょ。