仕事で詳細設計と実装をやっていたり、後輩のソースコードレビューをしたり。
そこで最近の悩み。
「良い設計・ソースとは」
私自身、”何が良い”という好みはある。けど、後輩に指導するときの根拠として弱いと思う。
そこで、皆さんのご意見をお聞かせ頂けたら、と思って今回のブログを書かせていただきたい。
今回は実際に私が困った事と類似の例を紹介させていただく。
【要件】
あるクラスの中で、0から9までの整数を格納しているリストが必要である。
今回はこのリスト取得のために、1つメソッドを作成したい。
メソッドは以下の要件を満たすこと。
- アクセス修飾子:private
- メソッド名:createList
【納品物1】
private void createList(List<Integer> list) { for(int i = 0; i < 10; i++) { list.add(new Integer(i)); } }
【納品物2】
private List<Integer> createList() { List<Integer> result = new ArrayList<Integer>(); for(int i = 0; i < 10; i++) { result.add(new Integer(i)); } return result; }
どちらも要件を満たしているが・・・
- どちらが”良い”ソースだと思いますか?
- それはなぜですか?
Facebookにもリンクを貼っていて、ご回答を頂きました。
返信削除その回答を転記したいと思います。
<1人目>
私は2番目ですね。
1番目だと、引数がnullの時に例外が発生するということと、
既に何かの値を追加しているListを渡した時にも追加されます。(想定外の動作の可能性)
まあ、用途によって、上記の懸念はなくなるのかもしれませんが。
<2人目>
既に値が追加されているリストに追加するような使い方を考えているかで、1番目か2番目か変わるかなと思います。
想定外であるならば、2番目です。
<3人目>
自分はcreate~というのを見ると、ファクトリパターンを想像するので2番目かなぁ。まぁ、結局はどこまでをメソッドの責務としたいか(Listの実体を呼び出し元で意識させる・させたくないとか)で、ケースバイケースかも。
1人目と2人目の方にご指摘頂いた「既に値があり、値を追加する」という処理は想定外の動作でした。
削除3人目の方の「ファクトリパターン」とは私の考慮にはありませんでした。
また、この方が仰る通り、結局はメソッドの責務によって変わってくるかと思っています。責務に関していうと、1人目の方のnull判定も関わってきますよね。