お恥ずかしいコード。
今日、職場で同僚とどっちが本当は良いのかな?的な議論がありました。ちょっとだけ楽しかったのと不勉強を実感したのでメモしておきます。
関数(メソッドでも良いのですが)の作り方です。
例えば、既存のレコードに対して更新処理を行う為のメソッドがあったとして、その中でxxxの場合、とある列に0をセット、そうでない場合は1をセットしなきゃいけない、とした場合。。。
- 更新処理を行いたいプログラムは、関数Aを実行。
- 関数Aの中で条件分岐
- 関数Aに渡されたパラメータの中身が空またはnullだった場合
- 顧客テーブルの列「偽装」に0をセット。(偽装なし)
- 関数Aに渡されたパラメータの中身が「カツラ」だった場合
- 顧客テーブルの列「偽装」に1をセット。(偽装あり)
こういった仕様で関数(だかメソッドだか)を作ってねー、というシチュエーションです。
パターンA
public void detectTarget(String[] args) { if (args[0] == null || args[0].equals("")) { // 偽装=なしで更新 updateRecord(args[1], Integer.parseInt(args[2]), 0); } else if (args[0].equals("カツラ")){ // 偽装=ありで更新 updateRecord(args[1], Integer.parseInt(args[2]), 1); } } private void updateRecord(String name, int age, int camouflage ) { // あっぷでーと こきゃくりすと せっと なまえ=name,ねんれい=age,偽装=なし }
という実装が良いんでないか?という意見と、
パターンB
public void detectTarget(String[] args) { if (args[0] == null || args[0].equals("")) { // 偽装=なしで更新 updateNature(args[1], Integer.parseInt(args[2])); } else if (args[0].equals("カツラ")){ // 偽装=ありで更新 updateCamouflage(args[1], Integer.parseInt(args[2])); } } private void updateNature(String name, int age) { updateRecord(name, age, 0); } private void updateCamouflage(String name, int age) { updateRecord(name, age, 1); } private void updateRecord(String name, int age, int camouflage) { // あっぷでーと こきゃくりすと せっと なまえ=name,ねんれい=age,偽装=なし }
の方が良いんじゃないか?という意見が出ました。(ちなみに私は後者支持です)
争点は、
- コードの量が多くなるのはどうなんだ?(A=12行、B=18行)
- メソッドの数が多くなるのはどうなんだ?(A=2つ、B=4つ)
- エントリ・ポイント(上記例の場合detectTarget)になるメソッドに妙なコードを意識させるのはどうか?
- パラメータかメソッド名の違いはあれど、エントリ・ポイントは使用するメソッドに依存してしまう為、結合度は下がらない。ならば、一目見てイメージし易いコードにしておいた方がよいのでは?
- "カツラ"検知時に限り、テーブル「こきゃくりすと」の他に「KGB候補者」というテーブルにもデータを追加しなきゃいけなくなった場合、(updateCamouflageメソッドにだけ手を加えれば良い)後者の方が良いんでは?
といったところです。
私の意見は、「コードの量が増える、メソッドの数が多くなるという事のどこが問題なのか、エントリ・ポイントに妙なコードを意識させるくらいなら、人目で分かるメソッド作ってしまえば良いじゃない。コードの量が増えたといっても、メソッドの中身は非常にスッキリしたんだし。多分デバッグもしやすいよ。」でした。
残念なのは、例えばコンパイラが最適化した後の状態がどうとか関数ポインタがどうとか、明確な根拠を示すだけの知識を私が持っていなかったので、きちんとした結論に達しなかったところです。
実際はどっちが良いんでしょう?