業務でJavaを使う際に気をつける事
今年から業務でプログラミングをするようになった。 言語はJava 13、FWはSpring Bootという最近のポピュラーな組み合わせでAPIを作った。 最初、学生に毛が生えたレベルだったので、よく書けているように見えて結合するとバグがポンポン出る感じだった。
書いてはバグ出て、それを直してを4ヶ月ぐらい続けた。 せっかくなので、その経験を通して学んだ事を書き留めておこうと思う。順番は適当。 レビュワーや反面教師、一緒に頭捻って考えてくれた方々には感謝。 ただ、これ自体も実はバッドノウハウかもしれない。なので、誰か指摘してれると嬉しい。
目次 [:contents]
外からきた値はNullである可能性を考慮する
基本的にはユーザも他の開発者もDBの中身も信じない。 信じ切ってしまうとNullPointer Exceptionが待っている。信じるものはすくわれる。 自分はDBを疑っていなかった。結合試験の初期だとDBの中身や設定が想定通りではないのですくわれた。
Nullを検査するのには大抵以下の4つを使うようにしていた。Nullでも安全に使える優秀なメソッドたちだ。
Objects.equals()
Objects.isNull()
Objects.nonNull()
org.apache.commons.lang.StringUtils.isEmpty()
org.apache.commons.lang.StringUtils.isNotEmpty()
ちなみに、Spring BootにもStringUtilsがあるけど、FW内部で使うために用意しているものなのでApacheの方を使うのが良いそうです。
安易に外からきたオブジェクトをメソッドチェーンをしない
例えば以下のようなコードを考えた時、リクエストボディに必ずしもメールアドレスは入っていないかもしれない。 DTO(model)の定義上はStringクラスになっているので、IDEの補完機能を使って安易にStringクラスのメソッドを繋げてしまうかもしれない。 でも、そうするとNullPointer Exceptionによるバグの元になる。
var foo = requestBody.getEmail().trim();
Optionalクラスで実装されているならOptionalクラスのメソッドを駆使して上手いこと書けばいいと思う。 そうじゃ無い場合、自分は以下のように書くように癖づけてた。
var foo = requestBody.getEmail(); foo = (Objects.nonNull(foo)) ? foo.trim() : foo;
他所の人では定数はNullにならないので以下のようなやり方してる人もいた。
個人的には定数なのにメソッドが続くのは見慣れず違和感が合った。
Objects.equals()
の方が汎用性のある書き方でバカの一つ覚えで行けるので優れている気がしている。
if (CONSTANT_VAL.equals(foo)) { ... }
!を使わないで書く方法を考える
Notとか書ければいいんだけどJavaにはそんな気の利いた演算子はない。
!は細くて見辛いし直感的じゃないのが嫌い。
!Objects.equals(foobar, null)
よりもObjects.nonNull(foobar)
の方が分かりやすい。
実際、自分は今前者を書いてても何回も確認してしまった。間違えてそうで怖い。
三項演算子を効果的に使う
2つ前の例で登場してムムッてなった人もいるかもしれないけど、三項演算子をうまく使うと逆に読みやすくなると思った。 当初は三項演算子は否定派だったけれども、逆にif文で書こうとすると冗長に感じてしまう。三行で同じじゃんって感じもするけど。 勿論、三項演算子で書く場合はワンライナーで書くと読みづらいので改行を駆使するのがいいと思う。コーディング規約違反ならごめん。
- 三項演算子の場合
var foo = requestBody.getEmail(); foo = (Objects.nonNull(foo)) ? foo.trim() : foo;
- if文の場合
var foo = requestBody.getEmail();
if (Objects.nonNull(foo)) {
foo = foo.trim();
}
嫌かもしれないけどコメントはとにかく書く
よくコードは読めば分かるように書くべきだし、読めばいいのでコメントは不要っていう人がいると思う。 正しいしそうあるべきだし、そうなるよう努力すべきだと自分も思う。 ただ、現場はそうじゃない人の方が大多数だし、何より書いてる本人もしばらく見ないと内容忘れる。 開発が終わった後の保守フェーズでは、参画する要員は単価が安いので能力も低い可能性が増えてくる。 現実路線で考えると書かざるを得ないと思います。
変数宣言は利用する処理の近くに記載する
自分の頭が古く、変数はクラスやメソッドの先頭にまとめて定義するものだと思ってたけど、可読性が下がるのでNGらしい。 確かに、変数名工夫しても「この変数ってなんの結果入ってたっけ?」ってなる事あるのでできる限り処理の近くに書く方がいいと思います。 人間、みんな鳥頭なんです。すぐキャッシュは消すんです。
定数管理クラスより列挙型を選ぶ
定数管理クラスをみんなで寄ってたかって更新すると以下のような問題が出る。
- 責任分界点が曖昧になる
- マージするとコンフリクトする
- 同じだけど別名の定数が何個もできる
列挙型なら意味単位でクラスが分かれるので上述の悪い点が全て解決する。
バリエーションがあるもの全て予め作っておく
業務仕様上バリエーションが存在するけど、今の段階のプログラムでは使う所が無いので定義しないでおこう。 こうすると、その経緯を知らない人はそれが定義されているものと思って開発を進めてしまい、結合するとバグになるみたいな事が起こり得る。 なので、例え今使っていないとしても仕様上バリエーションが定義されているのだから、仕様に従って定義する方がバグが少なくなると思う。
YAGNIの考え方とは逆行するように思うけど、あれは機能の話なのでこの話とは別だと思っている。
以上です。8個か。Javaと関係ない事も書いた気がするけど…まぁいっか。