arcanum_jp’s blog

おっさんの日記

コーディングの掟

 この本をチラ見して

コーディングの掟(最強作法) 現場でよく見る不可解なJavaコードを一掃せよ! (開発の現場セレクション)

コーディングの掟(最強作法) 現場でよく見る不可解なJavaコードを一掃せよ! (開発の現場セレクション)

 巡回先のブログに載っていたため、興味を持っていたんだけど本屋にあったので購入してみた。今日はパラパラと興味のある部分を読んで見た感想なんでまた書いてみるかもしれませんが。


 コードをレビューするのが頻繁な人にも、Javaを一通り組めるようになったけどオレのコードのどこに問題があるんだ!って人にも結構いい内容だった。でもどちらかと言うと開発者に規約や基盤クラスを提供、作った物を検証する側にとって有用なんじゃないかな。そういった人はコードを見る機会がたくさんあるはず。


 自分はどちらかと言うと前者的な感覚で読んだけど、第1章はまさにそんな感じ。「無意味な定数定義」の節や「巨大な共通定数クラス」の節なんかは、リテラルは悪!って思考が停止した人にはなんでまずいかが説明されています。


 たまにいますよね。全てのリテラルは定数にならなきゃならないって使命に燃えた人や、定数はアプリ全体で管理されるべきだ!って管理好きな勘違いした人が。そんな人にどうぞ。


 「不思議なJavaDocコメント」これは分かります。プロジェクトでコードインスペクションをどうするかって話になると「最近はIDEでチェックできるよ」って話になって、目でのコードインスペクションをおろそかにするところがあったりする。その際にEclipseなんかだとCheckStyleなんかでどのルールを適用するかって話になるとそこで思考停止して「便利なんだから全部やりゃいいじゃん」って話になりがち


 こういった機械チェックに対する自分の見解は「あの膨大な国際的なルールはあなたのローカルでちっちゃいアプリには全部必要なんですか?まずどれが必要で不要か考えましょ!それが開発者に開発環境を提供する側の仕事でしょ。」です。


 コレを怠るとJavaDocなんかは設計書がキチっとあるプロジェクトなんかには本来不要でもあるに係わらず作れ!って話になってIDEが勝手に作ってくれるJavaDocタグにテキトウな値をいれてチェッカーのチェックを逃れるってのが筋書き。


 ここからはJavaDocの問題とは関係ないけどIDEでのチェックはどうしてもイディオムのチェックはできても「なんで開発者がそうしたか?」って疑問はチェックできない。このコードに落ち着いたのは開発者にとって何か意味があるはずってのが自分の考え。そこは目でやるべき。


 昔のインスペクションのようにそのコードが業務プロセスを確実に実行できるかなんて所までやらずとも無駄なコードや不安なコードはある程度排除できる。(どうあがいても完全に排除できないってのが悲しいですけどね)そうすると「無駄なインスタンス生成」って節の話題にはこうなって簡単に発見できますよね。

List list = new ArryaList();
 ...
list = dbInfo.getSelectData(cond);

 本の中で紹介されたこんなコードは良く見かけます。

 自分「ここでListのインスタンスを直接生成して最終的に置き換えているけど何か意図はありますか?」

 回答A: 開発者「なんとなくです。」
 回答B: 開発者「サンプルがそうでした。Bさんのコピペがそうでした。云々・・・」
 回答C: 開発者「業務プロセスを実行するため、戻り値に配列0以上を返す必要があった」

 回答Cは(妥当であれば※この場合はどこかのタイミングでインスタンスが書き換えられるので甚だ疑問ですが)いいのですが、回答Aの場合(結構コレが多い)IDEコンパイラの機械チェックでは「変数hogeは初期化されていません!」とか言ったエラーや警告がでて開発者は「何か気持ち悪いんでnullかとりあえずのインスタンスを設定」を行い、はれてIDEコンパイラは何も言わなくなったってのが筋書きだろうと思う。昔自分もやったパターンなのである程度分かる。でも回答Bにいたっては規約やBさんのコードレビューで発見できなかったなど、サンプルやコードをチェックする側の完全なミス。


 この本での最大の収穫は、Javaで組む場合の修飾子はパッケージデフォルトを基本とすること。これに尽きます。自分も修飾子を付けないのが気持ち悪くてよく規約なんかには「必ずつける」見たいに書いてしまいますが、思えばパッケージプライベートが妥当だって感じられますね。


 例えば、Clickなんかを使っていてPageクラスのサブクラスを作った場合、イベント系はフレームワークが参照するのでpublicでなければならない。だけど、そのページが使うメソッドなんかは考えずにprivateにしがちなんだけど、たいていの場合パッケージ設計もしてアプリを組んでいるので、そのページクラスがあずかり知らぬクラスによってインスタンスを生成されたって同一パッケージじゃなけりゃアクセス不可なんだからメソッドがprivateである必要って言うのは無いのかなと。


 そのほか色々書いてみたいことがあるけど、次の機会があったらにしてみます。色々面白い話題があるので、読んでみるといいよ。マルチスレッドやロギングの章なんかは結構面白いと思う。