ソースコードを書く際のチェックポイント

# ソースコードのチェックポイント
自身がソースを書く時もそうだが、人のソースをチェックするときにも必要になる観点を備忘録としてまとめておく。

## マジックナンバーを使っていないか
```java
if (count > 10) {
  return "err_msg";
}
```
#### 何が悪いのか
:::note alert
上記のソースでは「10」「err_msg」が何を表すものであるのか直感的にわかりません。
定数やenumを使うなどしてわかりやすくしましょう。
:::

#### 修正

```java
final int MAX_USER_COUNT = 10;
final String ERR_MSG_OVER_MAX_USER_COUNT = "err_msg";

...(中略)...
if (count > MAX_USER_COUNT) {
  return ERR_MSG_OVER_MAX_USER_COUNT;
}
```
#### どう良くなったか
:::note info
if文の条件を読んだときに、ユーザーの上限を超えた時にエラーメッセージを返すことがわかるようになりました。
また、どういったエラー内容が返るのかイメージも付きやすくなりました。
:::

## フォーマットがかかっているか
```java
            private String manpukuStatus(int amount) {
        if (amount > STATUS_FULL) {return "満腹";
    } else if (amount < STATUS_MAX_NORMAL&&10 < STATUS_MIN_NORMAL) {return "腹八分目";
}else{return "おなかが減っている";}}
```

#### 何が悪いのか
:::note alert
読むとなったときに、どういった条件で分岐をして何を返しているのかわかりません。
:::

#### 修正
```java
    private String manpukuStatus(int amount) {
        if (amount > STATUS_FULL) {
            return "満腹";
        } else if (amount < STATUS_MAX_NORMAL
                && 10 < STATUS_MIN_NORMAL) {
            return "腹八分目";
        } else {
            return "おなかが減っている";
        }
    }
```

#### どう良くなったか
:::note info
if文での分岐やリターンなどが見やすくなりました。
:::

## true/falseをreturnするような形で記載されていないか
```java
if (age == 20) {
    return true;
} else {
    return false;
}
```

#### 何が悪いのか
:::note alert
boolean値を返すだけのための分岐が記載されてしまっています。
:::

#### 修正
```java
return age == 20;
```

#### どう良くなったか
:::note info
`age == 20`が成り立つときすでに`true`であることが確定しています。
このことは自明なので、冗長な記載にならなくて済みます。
:::

## 一度しか使っていない変数を定義してしまっていないか
```java
int status = jobStatus;

if (status == 0) {
  return NORMAL;
} else {
  return ERROR;
}
```

#### 何が悪いのか
:::note alert
一度しか使っていないのに変数が定義されることになってしまいます。
:::

#### 修正
```java
if (jobStatus == 0) {
  return NORMAL;
} else {
  return ERROR;
}
```

#### どう良くなったか
:::note info
不要な変数の定義がなくなりました。
:::

## 条件式はメソッド化する
```java
if (age >= TWENTY_YEARS_OLD && sex = MEN) {
...
}
```

#### 何が悪いのか
:::note alert
条件が簡潔な場合はあまり問題になりませんが複雑化すると条件を深く読み込まなくてはならなくなります。
:::

#### 修正
```java
if (isAdultMen()) {
...
}

```

#### どう良くなったか
:::note info
メソッドとして切り出すことで条件式に記載されているものをそのまま読めば条件分岐の細かい中身を知らなくとも理解がしやすくなります。
:::

## 固定値を引数としたメソッドはメソッドを作る
```java
mthod(MyEnum.FOO);
mthod(MyEnum.BAR);
```

#### 何が悪いのか
:::note alert
固定値のため、処理内容が変わらないのにメソッドの中身を細かく見ないと処理がわかりにくいです。
:::

#### 修正
```java
mthodFOO();
mthodBAR();
```

#### どう良くなったか
:::note info
FOOやBARに関連する処理であることがメソッド名でわかりやすくなります。
mthodFOO/mthodBARも簡潔になります。
:::

## 処理と定義がまとまっているか
```javascript
function gohan() {
  itadakimasu(); 
  taberu();

  function itadakimasu() {
      console.log("itadakimasu");
  }
  function taberu() {
      console.log("taberu");
  }
  gochisousama();
  function gochisousama() {
      console.log("gochisousama");
  }
}
```

#### 何が悪いのか
:::note alert
「gohan」を実際に呼び出した際には「itadakimasu」「taberu」「gochisousama」がコンソールに出力されます。  
しかし、「gohan」を参照した人は「itadakimasu」「taberu」で処理が終わるように見えます。  
実際には読み込めばわかるのですが、混乱をさせやすいです。
:::

#### 修正
```javascript
function gohan() {
  itadakimasu(); 
  taberu();
  gochisousama();

  function itadakimasu() {
      console.log("itadakimasu");
  }
  function taberu() {
      console.log("taberu");
  }
  function gochisousama() {
      console.log("gochisousama");
  }
}

```

#### どう良くなったか
:::note info
「gohan」を呼び出した際に「itadakimasu」「taberu」「gochisousama」が順に呼び出されて処理が終了することがわかりやすくなります。
また順序としても「gohan」での処理とその処理で使う定義で分かれているため理解がしやすくなります。
:::

## if文で同列ではない条件を比較していないか
```java
if (isMen()) {
...
} else if (isAgeOverThirty()) {
...
} else if (isLivingInJapan()) {
...
}

```

#### 何が悪いのか
:::note alert
男性であるか、女性であるか、未解答やその他であるかという観点
年齢が30歳を超えているかという観点
済んでいるところが日本であるかという観点
がごちゃごちゃになっています。
例えばここに年齢が20歳以上であるかという条件文が増えるとどんどん条件は複雑化していきます。
:::

#### 修正
```java
if (isMen()) {
...
}
if (isAgeOverThirty()) {
...
}
if (isLivingInJapan()) {
...
}
```

#### どう良くなったか
:::note info
それぞれどの属性によってif文が記載されているかもわかりやすくなりました。
そして何かの条件を追加したとしても、ある程度の明快さは担保できると思います。
:::

## 無駄に処理を書いていないか
```java
return new ArrayList<>();
```

#### 何が悪いのか
:::note alert
Collectionsで提供されているメソッドを使っていない。
:::

#### 修正
```java
return Collections.emptyList();
```

#### どう良くなったか
:::note info
個別でリストを作成する必要はなくなります。
インスタンス化が不要です（Collectionsクラス側に定義されているstatic finalの空リスト）。
:::

## 同一の比較ができているか
```java
list.isEmpty();
list.size() == 0;
```

#### 何が悪いのか
:::note alert
同じ意味であるはずだが、記述がぶれている
:::

#### 修正
```java
list.isEmpty();
```

#### どう良くなったか
:::note info
比較方法の統一で記載がわかりやすくなります。
:::

## 引数が固定値の場合のメソッド
```java
run(XXEnum.FOO);
run(XXEnum.BAR);
```

#### 何が悪いのか
:::note alert
固定値の場合は別途メソッドを作成する
:::

#### 修正
```java
runFoo();
runBAR();
```

#### どう良くなったか
:::note info
何に対しての処理であるかわかりやすくなります。
:::