新卒くんのコードレビュー①

はじめに

春ですね。もうすぐGWですね。心が躍りますね。
皆さん、GW明けの鬱には気をつけてくいださいね。
ということで新卒くんのコードレビューで感じたことを書いていきます〜
今回はswiftです。

正解だけど正解ではないif式

// flagはBool型
if (flag1 || flag2) {
    // 何かの処理をする
}

以上のような条件式があったとする。
実際はBool型ではなかったが、今回は分かりやすくBool型にしています。
新卒くんは、ある機能を修正するタスクを任されており、内容としてはflag2の条件を無視して欲しいというものだった。
それなら以下のようになるだろう。

// flagはBool型
if (flag1) {
    // 何かの処理をする
}

だが、新卒くんは以下のようなソースをプルリクエストしてきた。

// flagはBool型
if (flag1 || flag2 && flag3) {
    // 何かの処理をする
}

なぜか、条件が一つ増えている。
推測にはなるがflag1などが、どのような使われ方をしているか確認していなかったのであろう。。。
なのでflag1とほぼ同じ意味であるflag3を作ったのかなと。もはやif (flag3)だけでもいい。。。
これは一応動いていて、要件を満たしているが後々見た人はどう感じるだろう。
読みづらいかつ、バグが起こり得る箇所になってしまいます。
流石にコメント。

終わりに

自分も似たような間違いをいっぱいしてきたので、人のことは言えない。
でも新卒くんのコードレビューをして考えたことがいっぱいあるので残しておく。
初めての新卒くんのコードレビューなのに春を実感したって話。