正しいプログラミングなんてない

仕事で他の人が書いたソースのチェックをしていたんですが、その中にこんなのがありました。

public UserDTO selectUser(UserDTO[] userDTO){
    Vector<Object> vector = new Vector<Object>();
    for(int i = 0; i < userDTO.length; i++){
        if(条件を満たしているか){
            vector.add(userDTO[i]);
        }
    }

    UserDTO[] retUserDTO = new UserDTO[vector.size()];
    vector.copyInto(retUserDTO);

    return retUserDTO;
}

(もちろん、実際のソースそのままではなくイメージです)


なんでVector・・・?
同期化が必要な場面ではないし、ジェネリクスを潰してる。


ListとVectorの違いは、同期化の有無*1とaddで容量不足した時に拡大する量の違い*2ぐらいのハズ。
ここではListを使うべきで、Vectorの必要はなさそう・・・。
型にObjectを使うメリットは、もし型が変わったとしても楽に変更ができる・・・ぐらい、たぶん。


自分なら、こう書いてると思う。

public UserDTO selectUser(UserDTO[] userDTOs){
    List<UserDTO> list= new ArrayList<UserDTO>(userDTOs.length);
    for(UserDTO userDTO : userDTOs){
        if(条件を満たしているか){
            list.add(userDTO);
        }
    }

    return list.toArray(new UserDTO[list.size()]);
}

(8/3修正 new UserDTO[0] -> new UserDTO[list.size()] thanks! id:onozaty)


ただ、元のソースでも動くし、このメソッドの引数で渡される配列の中身も多くて数百ぐらい。
だからちょっと処理効率が悪くても、いいのかもしれません。
ちょっと悩んで、近くに来ていた上司に相談して、結局このままにしました。


正しいプログラミングなんてないし、どれが「いいプログラム」かなんて言えない。
経験不足な面もあって、自分の中でも「これがいい!」と思ってやった3か月前のプログラムも、
今見なおすと「・・・変なこと書いてる」。
だから、いいのかな、と思う。


でも・・・自分としてはやっぱり「正しいプログラミング」をしようと、日々精進していきたいと思いました。



ちなみに。
この処理、そもそもデータベースから取ってきている値をふるいにかけるぐらいなら、
最初からWHERE句で条件指定した方がいいという点もあるんですが・・・
これはちょっとまた別の問題があるので、また後日。

*1:同期化コストの関係でArrayListの方が速いと言われている。手元の環境で試したら、ArrayListの方がVectorより1.5倍ぐらい速い(初期容量をあとで拡大する必要がないぐらい十分に取っていた場合)。ちなみに、ArrayListをCollections.synchronizedListでラップしてみたけど、やっぱりArrayListの方が1.3倍ぐらい速い・・・あれ?

*2:Vectorが元の容量の2倍(ただし変更可)、ArrayListが1.5倍に内部の配列を拡大する