【Java】新卒が初めてのコードレビューで受けた指摘５選

はじめに

新卒でSIerのJavaプログラマとして働き始めて２ヶ月が経ちました。
忙しい中、自分の拙いコードを見てくださる先輩方には感謝しかありません。

今後同じ指摘を受けないように、コードレビューでの指摘をまとめようと思います。

①Nullぽを起こさないようにチェックを入れる。

    Customer customer = getCustomer(request);
    System.out.println("ようこそ、" + customer.getName() + "さん");

もしgetCustomerで顧客情報が取得できなかった場合、customerはnullとなり、getNameメソッドを呼び出す時点でヌルポが発生します。

Nullチェックするときは、CollectionUtils.isEmptyとか、StringUtils.isEmptyとかを使いましょう。

②Stringのequalsメソッドで比較する際は、定数を左に持ってきてヌルポを回避する

    String answer = getAnswer();
    
    if(answer.equals("A")) return "正解です";  // (1)
    if("A".equals(answer)) return "正解です";  // (2)

(1)の場合は、仮にgetAnswerで答えを取得できずanswerにNullが入ったらヌルポが起こります。
(2)の場合は、左側に定数があるためヌルポは起こりません。

③マジックナンバーは使わない

数値とか、”01”みたいな文字列は、後々何を表すか分からなくなります。
以下のコードは、なんとなく”01”と”02”が春夏で、”03”と”04”が秋冬であると察せますが分かりにくいです。また、別の箇所で”01”が、全然違った意味で使われた場合、確実に混乱します。

    String seasonNo = clothes.getSaleSeasonNo();
    
    if("01".equals(seasonNo) || "02".equals(seasonNo)) return getThisYear() + "SS";
    if("03".equals(seasonNo) || "04".equals(seasonNo) ) return getThisYear() + "AW";

マジックナンバーには名前をつけてあげましょう。

    private final String SPRING = "01";
    private final String SUMMER = "02";
    private final String AUTUMN = "03";
    private final String WINTER = "04";

④定数を作る

プログラム内で変わらない値がある場合は、定数として定義します。
必要であれば、どんな定数かコメントアウトをつけましょう。
また、変数名は大文字にします。

    public class Sample {
      private static final String GREETING_MESSAGE = "Hello, world!";
      System.out.println(GREETING_MESSAGE);
    }

また、様々な場所で使われる定数の場合は、切り出しましょう。

constsパッケージ内のConstクラスに書けばいいかなと思っていましたが、Enumクラスというものを初めて知りました。

※余談 private static finalについて目に止まった記事

⑤いらないelseを書かない

シンプルにこちらの記事がとても分かりやすかったです。
https://refactoring-brique.info/logic/else/