なぜリテラルを後にするのか

http://d.hatena.ne.jp/nowokay/20081208#1228726053

http://d.hatena.ne.jp/nowokay/20081204#c1228454951

別にリテラルを後にしたコードが美しいとかカッコいいからそうしているわけじゃなくて、単純にコードとして優れていないから選択すべきではないというのが僕の考えです。

 if("リテラル".equals(s)) {..}

↑のコードみたいにリテラルを先にした場合、2つの点でメンテナンス性に問題が発生します。

  • nullチェックがパスされてしまうという余計なコンテキストが発生する。
  • nullableかどうかがあいまいになるので、ロジックが不安定になる。

リテラルを先にした場合、nullチェックがコード上で表現されないので、メンテナンスするときに「nullチェックがパスされている、nullチェックがパスされている、nullチェックがパスされている」と念仏を唱えながら修正する必要がある。

隠されているコンテキストに気づかなかったり、念仏を唱え忘れたりすると↓のような修正をして世の中のバグがひとつ増える。

 if("リテラル".equals(s)) {
  ..
 } else {
  s.substring(x,z); // 当然ヌルポ
 }


こういう単純なミスは初心者だろうとベテランだろうとやってしまいがちで、修正が単純なだけにテストするのを忘れられたりする。

これがリテラルを後にして書いていた場合にどうなるかというと、

 // 事前にnullでないことが保障されている場合。
 if(s.equals("リテラル")) {
  ..
 } else {
  s.substring(x,z);
 }

 // nullがありえる場合
 if(s != null && s.equals("リテラル")) {
  ..
 } else if(s != null){
  s.substring(x,z);
 }

隠れているコンテキストを見抜く必要も念仏を唱える必要もない。nullableな場合もnullチェックが表現されているので気づきやすい。nullableでない場合は事前にnullでないことが保証されているのでそのまま安心して修正する事ができる。

2つめの問題は狭いスコープの中でnullableなのかどうかが判断できない点。

 if("リテラル".equals(s)) {..}

リテラルを先にした場合、狭いスコープの中でnullableかどうかは判断できないから、上の例みたいにelseを追加したたり、ifの内部で参照しようとすると、nullableかどうかを確認してから修正する事になる。つまりリテラルを先にすると修正時のステップが確実に一つ多い。

さらにnullableかどうかをあいまいにすることで、無駄なnullチェックを拡散することになる。そのため問題が発生すべきところで発生しないで全然関係が無いところでエラーが発生して問題を深くすることもある。ついでにロジックも曖昧になるのでバグを増やすという効果もある。

なのでnullチェックを省略できるからとか、ちょっと見た目がカッコ良さそうとか、誰かがそう言ってたからという理由でリテラルを先にしている人はちゃんとにメリット・デメリットを計ってみるべき。リテラルを先にするとトレードオフが見合わないはずなので。

次にリテラルを先にした状態で上の問題に対処してみる。

1.コメントに書いておく

 // sはnullの場合もある。修正する場合はnullチェックを忘れずに!
 if("リテラル".equals(s)) {..}

まあ、バカバカしい。

2.リテラルを先にする場合は常にnullチェックを書くようにする。

 if(s != null && "リテラル".equals(s)) {..}
 if(s == null && "リテラル".equals(s)) {..}

これは確かに悪くない方法だけど、さすがに冗長すぎるので、どっちかを省略する規約を作った方がいいかも。そんな手間をかける価値があってさらに規約を守ってくれる希望があればの話だけど...

2の方法が取られていればまだ許容できますが、そんなコードは見たこともないのでリテラルを先にしているコードはメンテナンスをする人に余計なコストを押し付けていることになるので「手抜き」と判断してしまって問題ないと思います。


ちゃんとしたシステムを作りたいならきちんとテストをすべきで、リテラルを先にしたほうが安全という考えはテストの戦略が間違っていることを表しているのでは?リテラルが先だろうと必要なテストケースの数は変わらないはずだし、テストが甘ければユーザに迷惑をかけるのは当然なので。

可読性を優先させるためにリテラルを先にするという意見にもちょっと賛成できないです。何のために可読性を確保するのかといえば、一番にはメンテナンス性を確保するためだから。メンテナンス性を犠牲にして可読性を確保するのはふつーに考えておかしい。

結局の所どちらを選択すべきかはソースコードを書くときに何を優先しているか?というところだったりするわけで...

僕は

  • 頭の中にかかるコストを最適化するようなコードを書く。
  • 問題は可能な限り局所化する。
  • 問題が発生してもできるだけわかりやすくしておく。
  • コードで説明可能なものはコードで説明する。

というのをプライオリティの高いほうに置いているので、リテラルを先にするという選択肢はありえない。

ついでに、この問題の根本的な原因は文字列を意味も無くnullで初期化することだったり、DBがnullと空文字を区別しないくせしてJDBCにnullの場合に空文字を返すようなオプションがないせいだったりする。

もっと根本的にはオブジェクト指向言語がインターフェース間でnullをやり取りするのがC言語メモリリーク並みに問題を引き起こしているという事実もある。すでに学習に必要なだけのNullPointerExceptionはthrowされてるはずなので。

書きかけで下書きに置いておいたけど、近くにリテラルが先の方が正しいとか言う人がいたので最後まで書いてみた。