The Backyard - HowToUseConstantsOrLiteral Diff
- Added parts are displayed like this.
- Deleted parts are displayed
like this.
!数値そのものに意味がある場合は直定数を利用すべき
ダメなコードとは
const int ZERO = 0;
...
int foo = ZERO;
みたいなやつのことだ。
たまに数値そのものに意味があるのにわざわざ定数にしてあるのを見るとうんざりした気分になる。
定数にするのはあくまでも名前に意味がある場合に限定すべきだ。たとえば
const int DEFAULT_TCP_DELAY_ACK = 500;
とか。これは500には意味がないからOK。
つまり値が可換(名前に意味がある)なら定数、そうでなければ直定数ということだ。
ちなみに、
int foo = NULL;
...
foo++;
のようなのを見るがこれはもっと悪い。++しているということは数値として
利用しているということだ。こういう場合にNULLを利用してはいけない。
int foo = 0;
が正しい。
何もわかっていないと「直定数禁止」みたいなコーディング規約が出てくることがあるが、やめたほうが良い。
はしにもぼうにもかかわらないレベルを想定し過ぎると可読性が極端に落ちる。
たとえば平均50点のメンバーがいるとして、平均を規準としたコーディング規約を作る（上の「直定数禁止」のような0点ではないがせいぜい50点ルールということだ）と、確かに50点のコードができるかも知れない。
しかし良く考えてみよう。30点のメンバーは全体のコードの数パーセントくらいしかそもそも書けないのだ。一方80点のメンバーは全体の数10パーセント(へたすればほとんど全て)を書くだろう。であれば、80点から100点ルールを決めれば、ほとんどのコードが非常に優れたコードになることは自明である。彼らのポテンシャルを低いほうに合わせてはならない。
プログラムを書きも読みもしない人間にコーディング規約を決めさせてはいけない。
正しくは、「数値そのものに意味がある場合には直定数を利用する。そうでなければ定数を定義する。」とすべきである。または「可換性が要求される数値''のみ''定数として定義する。」である。
!!こんなのは最低だ。
const int ONE = 1;
const int TWO = 2;
const int SLEE = 3; // こういうのに限って妙な単語に出くわすね。
...
int step = throw_dice(); // サイコロの目をランダムに出す
switch (step) {
case ONE:
...
1は1、2は2、……6は6に決まってるじゃん。
つまり、
const int ONE = 1;
というのは
const int DEFAULT_TCP_DELAY_ACK = 500;
とは決定的に異なる。
DEFAULT_TCP_DELAY_ACKは、仮にそれが定説としてももしかしたら300になっても良いし、0になっても良い。
でも、ONEが1以外の値を取る必然性も可能性もない。
ちょっと考えればすぐにわかるが、
const int ONE = 3;
ってありえないわけで、あったらそれはバグである。
結局、数値をそのまま記述するのが正しい。
switch (step) {
case 1:
...
(例は悪いけどそういうことだ)
!どういう場合に定数を定義するか
ところが同じサイコロの目であっても定数にすべき場合もある。
const int BACK_TO_STARTPOSITION = 1;
const int GOTO_JAIL = 2;
...
const int STAY_HERE = 6;
これは現時点での（そして多分変わらないことが予測される）ルールだが、それでも可換である。
この場合6という数値に意味があるのではなく、「ここに留まる」という目が出たことに意味があるからだ。
!しかしまだ話は終らない
ところが（とまだ続く）、このように数値には意味がなく、名前に意味があってもやはり直定数を使うべき場合がある。ステートが複数になる場合だ。（注：以下では数値そのものはサイコロの目を意味していない。状態を表す数値となっている）
readonly int[] RUNNING = { 1, 2, 3, 4, 5, 6 };
readonly int[] IN_JAIL = { 6, 6, 6, 6, 6, 1 };
....
こういった場合に、それぞれのステートで取り得る値を別に定数にして、それをさらにステートテーブルに埋め込むとコードが長くなるだけで可読性が極度に落ちるからだ。
const int BACK_TO_START = 1;
const int GOTO_JAIL = 2;
...
readonly int[] LUCKY_POINT = {
BACK_TO_START,
GOTO_JAIL,
...
}
一見すると読みやすく感じるかも知れないが、それは誤解かまたは例のため行数が短いからである。
==それに、実際にはこの記述は意味的に正しくない。==(修正：[[中田さんの指摘で気付いた(感謝)|http://arton.no-ip.info/diary/20050218.html#c02]])
以下のクォートした部分は書いたときに感じたことのバイアスがかかっているから屁理屈です。ただし、表として記述する場合（というかこの例だと1次元配列だから成り立っていないわけだが）に、読みにくくなるという意見そのものを取り下げるものではありません。
ここではLUCKY_POINT[0]がBACK_TO_STARTとなっているが使うコードを想像してみよう。
if (LUCKY_POINT[dice_value] == BACK_TO_START) {
...
妙ではないだろうか？ すなわち、LUCKY_POINTが意味のある名前であって、その要素には意味はない（＝数値そのものに意味がある）からだ。
このような場合はテーブルの定義では直定数を記述し、各ステートでの取り得る値は別途コメントなどで表として記述するほうが良い。
!定数そのものを使わない
ここまでは、C++のようなC#のような妙なリストで記述していたが、以下ではC#として記述してみよう。
上の配列の例は実はあまり良くない。実際には次のようなコードとなるからだ。
const int BACK_TO_START = 1;
const int GOTO_JAIL = 2;
...
readonly int[] LUCKY_POINT = { 1, 6, 3, 2, 4, 5 };
...
int value = throw_dice();
switch (LUCKY_POINT[value]) {
case BACK_TO_START:
...
break;
case GOTO_JAIL:
...
break;
...
というコードよりも以下のようにデリゲートを利用したコードのほうがよりメンテナンス性は高くなる。
class Foo {
delegate void Move();
readonly Move[] LUCKY_POINT;
Foo() {
LUCKY_POINT = new Move[] {
new Move(BackToStart), new Move(GotoJail), ...
};
}
void BackToStart() {
...
}
void GotoJail() {
...
}
...
int value = throw_dice();
LUCKY_POINT[value]();
...
単純な美点は、元のリストではcase節の中に記述されていた（またはcase節の中から呼び出されていた）処理が、独立したメソッドとして分離されているため、個々の処理の独立性が高まることだ。（case内に処理をそのまま記述すればthrow_diceを呼び出すメソッドの行数はいやでも長くなってくる）
しかしそれ以上に重要な点は、後者のリストでは状態を示す定数がコードに含まれないということだ。
最初のリストでは状態を名前とその名前に関連した無意味な数値という組み合わせで表し、より重要な状態そのものが無名な処理で表現されている。それに対し、後者のリストでは名前を持つのは状態に応じた処理を実行するメソッドだ。これにより、必要な情報の分散（前者は定数定義、状態を示す配列、分岐ラベルの3ヶ所に出現、後者は状態を示す配列とメソッド定義の2ヶ所に出現）が少なくなっている。密接に関連する情報が複数箇所に分散していると、修正個所が増える（たとえばGOTO_JAILという状態をGOTO_POOLという状態へ入れ替える場合を想定してみよう。修正個所は後者のほうが少ない）が、それらの積み重ねが可読性やメンテナンス性を損なうことになるのである。
結局のところ、この例のように状態を示す値については定数か直定数かという選択はそれほど差がないのだ。
かくして2つめとして、「状態は直接その状態を処理するメソッドとして記述する」ということが言える。
ダメなコードとは
const int ZERO = 0;
...
int foo = ZERO;
みたいなやつのことだ。
たまに数値そのものに意味があるのにわざわざ定数にしてあるのを見るとうんざりした気分になる。
定数にするのはあくまでも名前に意味がある場合に限定すべきだ。たとえば
const int DEFAULT_TCP_DELAY_ACK = 500;
とか。これは500には意味がないからOK。
つまり値が可換(名前に意味がある)なら定数、そうでなければ直定数ということだ。
ちなみに、
int foo = NULL;
...
foo++;
のようなのを見るがこれはもっと悪い。++しているということは数値として
利用しているということだ。こういう場合にNULLを利用してはいけない。
int foo = 0;
が正しい。
何もわかっていないと「直定数禁止」みたいなコーディング規約が出てくることがあるが、やめたほうが良い。
はしにもぼうにもかかわらないレベルを想定し過ぎると可読性が極端に落ちる。
たとえば平均50点のメンバーがいるとして、平均を規準としたコーディング規約を作る（上の「直定数禁止」のような0点ではないがせいぜい50点ルールということだ）と、確かに50点のコードができるかも知れない。
しかし良く考えてみよう。30点のメンバーは全体のコードの数パーセントくらいしかそもそも書けないのだ。一方80点のメンバーは全体の数10パーセント(へたすればほとんど全て)を書くだろう。であれば、80点から100点ルールを決めれば、ほとんどのコードが非常に優れたコードになることは自明である。彼らのポテンシャルを低いほうに合わせてはならない。
プログラムを書きも読みもしない人間にコーディング規約を決めさせてはいけない。
正しくは、「数値そのものに意味がある場合には直定数を利用する。そうでなければ定数を定義する。」とすべきである。または「可換性が要求される数値''のみ''定数として定義する。」である。
!!こんなのは最低だ。
const int ONE = 1;
const int TWO = 2;
const int SLEE = 3; // こういうのに限って妙な単語に出くわすね。
...
int step = throw_dice(); // サイコロの目をランダムに出す
switch (step) {
case ONE:
...
1は1、2は2、……6は6に決まってるじゃん。
つまり、
const int ONE = 1;
というのは
const int DEFAULT_TCP_DELAY_ACK = 500;
とは決定的に異なる。
DEFAULT_TCP_DELAY_ACKは、仮にそれが定説としてももしかしたら300になっても良いし、0になっても良い。
でも、ONEが1以外の値を取る必然性も可能性もない。
ちょっと考えればすぐにわかるが、
const int ONE = 3;
ってありえないわけで、あったらそれはバグである。
結局、数値をそのまま記述するのが正しい。
switch (step) {
case 1:
...
(例は悪いけどそういうことだ)
!どういう場合に定数を定義するか
ところが同じサイコロの目であっても定数にすべき場合もある。
const int BACK_TO_STARTPOSITION = 1;
const int GOTO_JAIL = 2;
...
const int STAY_HERE = 6;
これは現時点での（そして多分変わらないことが予測される）ルールだが、それでも可換である。
この場合6という数値に意味があるのではなく、「ここに留まる」という目が出たことに意味があるからだ。
!しかしまだ話は終らない
ところが（とまだ続く）、このように数値には意味がなく、名前に意味があってもやはり直定数を使うべき場合がある。ステートが複数になる場合だ。（注：以下では数値そのものはサイコロの目を意味していない。状態を表す数値となっている）
readonly int[] RUNNING = { 1, 2, 3, 4, 5, 6 };
readonly int[] IN_JAIL = { 6, 6, 6, 6, 6, 1 };
....
こういった場合に、それぞれのステートで取り得る値を別に定数にして、それをさらにステートテーブルに埋め込むとコードが長くなるだけで可読性が極度に落ちるからだ。
const int BACK_TO_START = 1;
const int GOTO_JAIL = 2;
...
readonly int[] LUCKY_POINT = {
BACK_TO_START,
GOTO_JAIL,
...
}
一見すると読みやすく感じるかも知れないが、それは誤解かまたは例のため行数が短いからである。
==それに、実際にはこの記述は意味的に正しくない。==(修正：[[中田さんの指摘で気付いた(感謝)|http://arton.no-ip.info/diary/20050218.html#c02]])
以下のクォートした部分は書いたときに感じたことのバイアスがかかっているから屁理屈です。ただし、表として記述する場合（というかこの例だと1次元配列だから成り立っていないわけだが）に、読みにくくなるという意見そのものを取り下げるものではありません。
ここではLUCKY_POINT[0]がBACK_TO_STARTとなっているが使うコードを想像してみよう。
if (LUCKY_POINT[dice_value] == BACK_TO_START) {
...
妙ではないだろうか？ すなわち、LUCKY_POINTが意味のある名前であって、その要素には意味はない（＝数値そのものに意味がある）からだ。
このような場合はテーブルの定義では直定数を記述し、各ステートでの取り得る値は別途コメントなどで表として記述するほうが良い。
!定数そのものを使わない
ここまでは、C++のようなC#のような妙なリストで記述していたが、以下ではC#として記述してみよう。
上の配列の例は実はあまり良くない。実際には次のようなコードとなるからだ。
const int BACK_TO_START = 1;
const int GOTO_JAIL = 2;
...
readonly int[] LUCKY_POINT = { 1, 6, 3, 2, 4, 5 };
...
int value = throw_dice();
switch (LUCKY_POINT[value]) {
case BACK_TO_START:
...
break;
case GOTO_JAIL:
...
break;
...
というコードよりも以下のようにデリゲートを利用したコードのほうがよりメンテナンス性は高くなる。
class Foo {
delegate void Move();
readonly Move[] LUCKY_POINT;
Foo() {
LUCKY_POINT = new Move[] {
new Move(BackToStart), new Move(GotoJail), ...
};
}
void BackToStart() {
...
}
void GotoJail() {
...
}
...
int value = throw_dice();
LUCKY_POINT[value]();
...
単純な美点は、元のリストではcase節の中に記述されていた（またはcase節の中から呼び出されていた）処理が、独立したメソッドとして分離されているため、個々の処理の独立性が高まることだ。（case内に処理をそのまま記述すればthrow_diceを呼び出すメソッドの行数はいやでも長くなってくる）
しかしそれ以上に重要な点は、後者のリストでは状態を示す定数がコードに含まれないということだ。
最初のリストでは状態を名前とその名前に関連した無意味な数値という組み合わせで表し、より重要な状態そのものが無名な処理で表現されている。それに対し、後者のリストでは名前を持つのは状態に応じた処理を実行するメソッドだ。これにより、必要な情報の分散（前者は定数定義、状態を示す配列、分岐ラベルの3ヶ所に出現、後者は状態を示す配列とメソッド定義の2ヶ所に出現）が少なくなっている。密接に関連する情報が複数箇所に分散していると、修正個所が増える（たとえばGOTO_JAILという状態をGOTO_POOLという状態へ入れ替える場合を想定してみよう。修正個所は後者のほうが少ない）が、それらの積み重ねが可読性やメンテナンス性を損なうことになるのである。
結局のところ、この例のように状態を示す値については定数か直定数かという選択はそれほど差がないのだ。
かくして2つめとして、「状態は直接その状態を処理するメソッドとして記述する」ということが言える。