明日もう一度来て下さい。本当のマジックナンバーを見せてあげますよ
はじめに
下記の記事を見つけたのですが内容は大筋は賛成なのだけど、消費税をマジックナンバーという言葉で色々と違和感が。
これってマジックナンバーの話関係ある? と思ったのでちょっと自分の考えを整理してみました。
TL;DR
- (狭義の)マジックナンバーは使うな。常に名前を付けよう
- たとえ自明でもコード上で重複があるなら定数/変数や関数にしよう
- 共通ロジックを作ろう! でも気を付けながら。
マジックナンバーとは?
私の理解ではマジックナンバーというのは意味の分からない数値です。例では消費税率という少なくともコンテキストが分かれば意味の分かりそうなものが使わていますがコード値とかフラグ値のようなそもそも意味のない値に使うことの方が多い印象です。これが本当のマジックナンバーです。
例えば以下のような条件分岐を考えます。タクシー料金を見積もるコードです。金額は適当なのであしからず。
def taxi_fee code
if (code == 13)
1000
elsif (code == 11 or code == 12 or code == 14)
5000
else
10000
end
end
さて、このコードは意味が分かるでしょうか? 東京なら1000円、埼玉/千葉/神奈川なら5000円、それ以外なら1万円という料金です。安いな。。。乗りたい。
で、勘の良いというか知ってる人なら分かるでしょうがこれは都道府県コードです。都道府県コードという有名なコード値ですら余程じゃなければ空で読み書き出来ないと思います。ましてや社内の独自の謎の業務コードやアプリ独自のフラグと言うものは知らないと分からないし、たくさんの悲しみを超えないと暗記できません。障害対応とかひたすら業務で打ち込むとか。。。 なので意味の分かる名前を付ける これがマジックナンバー禁止の理由です。関数にしようがスコープを小さくしようが分からんものは分からん。それがマジックナンバー。一般的な用法では無いですが、この記事では 「狭義のマジックナンバー」 と呼びます。
こちらのコード値を定数化してみましょう。
def taxi_fee code
SAITAMA=11
CHIBA=12
TOKYO=13
KANAGAWA=14
if (code == TOKYO)
1000
elsif (code == SAITAMA or code == CHIBA or code == KANAGAWA)
5000
else
10000
end
end
格段と分かりやすくなりましたね? 大事なのは定数か変数かでもなく、メソッド内なのかトップレベルかなのかでもなく意味の分かる名前を付ける事です。ただ、一般的にスコープが広くなれば意味を一意にするために言葉を尽くさないといけないですし当然小さい方が良いですけれど。グローバルに使うならenumやそれに相当するオブジェクトやモジュールを作るのが良いでしょう。名前空間をより柔軟に区切れて便利です。数値じゃなくて型ならコンパイル等でチェックも出来ますし。
ようは 「(狭義の)マジックナンバーは常に使うな!」 ってことです。定数化(or 変数化 or 関数化)は役に立つのです。汝、名を授けよ。
なおもう1点大事なのが「意味のある名前」ということです。つまり
CODE_11=11
CODE_12=12
CODE_13=13
CODE_14=14
こういうのは意味が無いんじゃー! という事です。
消費税率はマジックナンバーなのか?
消費税、というかコンテキストを明確にすれば自明な値のハードコーディングはそもそもマジックナンバーと呼ばれるのか? これは議論の余地があるし私もモヤっとするけれど、マジックナンバーと呼ぶこともおかしくは無いようです。
Wikipediaにも載っていた消費税の例は分かりやすい解説です。
消費税は変わる事があるし事実として変わってきました。つまりコンテキストが失われる可能性があります。0.03で掛けていて昔の消費税だとすぐに気づけるでしょうか? 所得税の税率は自明ですか? 法人税は? このように人や時代によってコンテキストは容易に見失われます。正直、ちょっとモヤっとはするけど、確かに消費税も定義的に間違いなくマジックナンバーですね。マジックナンバーというかハードコーディングの問題
元記事で 「マジックナンバーをどんどん使え」 と言ってるのは 「その値の意味が自明であるなら名づけなど不要」 という事ですね。元より狭義のマジックナンバーの話をしていません。それならば一理あります。円周率を使う文脈で3.14と出たらそれは円周率です。プログラム上でわざわざ名前を付けなくても分かります。なので関数内などでコンテキストが明確化されているならリテラルを直接使っても良いと思います。だから元記事の最終結論のコードはあれで良いと思いますし、少なくともトップレベルに定数を書くよりもずっと良いでしょう。私ならローカル変数に入れるだろうけど、まあ、そこまで強くは拘らないですね。このくらいなら。でも3年後の自分が困らない用に強いこだわりが無いなら名前付けた方が良い。
ただし、こうした数値のハードコーディングには別問題もあります。それが記事のコメント等でも指摘されている改修が無限増殖する問題。Wikipediaにはマジックナンバーの問題の一つに上げられていますが、正直マジックナンバーに限らず文字列だろうが一連のロジックだろうが 「共通化できるコードを共通化しない」 場合には常に発生するので別の問題として捉えた方が考え方がシンプルになります。
具体的にどのような問題かというと今回の場合だと、例えば元記事では給与所得控除額の関数の中で収入金額 <= 1_625_000
を使っています。これはこの関数だけで考えれば問題ないです。ただ、仮にこの判定条件が他の関数でも使う必要が合ったりすると少し問題が出ます。その場合は同じ意味の1_625_000
を共通の定数なり変数なりで参照できるようにしておくことで共通化ができるので将来、法律が変わっても一括して修正が出来ます。まあ、その場合も数値だけではなく判定条件ごと別な関数に切り出す方が効果的なので元記事の切り出し方は少なくとも記事から見える範囲では妥当な気がします。画面表示とか別な所でもこの値使うなら別な話ですけどね。
ちなみに、このような 「コードを重複させない/改修箇所を限定する」 という考え方をOAOO(Once And Only Once)原則と呼びます。値だろうがロジックだろうが散らばってると整合性が取れないので、変数/定数にしたり関数やクラスにして重複を可能な限り無くせ、という考え方ですね。ただし名付けの問題を別にすれば、コード値とか以外の値は、さほど広いスコープで使われないと思うので、関数でロジックごと括り出す方がほとんどの場合で効果的です。
あと、ハードコーディングを厳密に避けて設定ファイルにした方が良いという話もあるけど、ちょっとこれは違う話題かな。
共通化の罠に注意
OAOOの原則という単語を知らなくても、重複するのは何となく悪い事で共通ロジックを処理する関数とかを作った方が良い気は何となくします。同じコードをいっぱい書くのは嫌ですしね? ただなんとなくでやると危険なので意識してやりましょう。
例えば
- 消費税の10%を増やす
- リボ金利で10%を増やす
みたいな処理をAdd10(x)
みたいな共通関数には決してしてはいけません。確かに実装されるロジックは一緒かもしれませんが意味が異なります。消費税やリボ金利が異なる値になった時にどうしますか? 共通化は同じ実装か否かではなく同じ意味かどうかでやる必要があります。
他にも似た処理だからと微妙な差分をフラグ値で吸収しようとするとメンテ不能の神の関数が生まれます。これも本当の共通ロジックをさらに抽出したうえで、それぞれの機能の関数を作ったりと適切な設計が必要です。
このように共通ロジックの開発というのは地味に難易度の高い作業なので、初心者のうちは先輩とよく相談したりきちんとレビューを受けながらやりましょう。
まとめ
- (狭義の)マジックナンバーは使うな。常に名前を付けよう
- たとえ自明でもコード上で重複があるなら定数/変数や関数にしよう
- 共通ロジックを作ろう! でも気を付けながら。
元記事の内容もだけどコメントも盛り上がってたのでちょっと自分の中の整理を書いてみました。それにしてもマジックナンバーってみんな数値のハードコーディング全般に使うんだなぁ。なんかそれは違和感。。。
あと、全く違う意味でファイルの識別子を指すこともあるみたいですね。そして魔法数はさらに別もの。
それではHappy Hacking!
Discussion
0.03ではないでしょうか。
ですね。消費税30%は地獄… 修正しました。ご指摘ありがとうございますー