はじめに

WEBサービス開発に関わって約6ヶ月のにこと申します！

さて、ここで質問です。

あなたは初めてコードレビューを受けたときのことを覚えていますか？

覚えていますよね？

・・・え、覚えていない？

そしたらこの記事はもしかしたらあなたが駆け出しエンジニアのあの時の頃を思い出させてくれるかもしれません・・・。

私はコードレビューを受けた時に衝撃を受けました。

こんな書き方が出来るとは・・・。私はなんとおブスなコードをかいていたんだろうと・・・。

レビューを受けてから良いコードを書けるようにより意識をするようになりました。（実際にきちんと出来ているかどうかはまた別のお話）

そしてこのコードレビューを自分の中だけで留めておくのはもったいない！！という気持ちと振り返りのためにまとめました。

これからエンジニアになろうとしている方、私と同じくまだ実務経験が浅い方、そして初心の気持ちを思い出したい方（？）に参考になる！！はずです！！

※言語はJavaScript(Vue.js)を使用

Boolean型を返す変数やメソッドの命名

👎Bad

// キャンペーンが有効かどうか
function campaignToggle () {
    // 略
}

• どのような値が返ってくるのかわかりにくい
• 確認するのに余計な手間がかかる

👍Good

function isCampaignActive () {
    // 略
}

👉Point
- Boolean型の変数名はその変数名を見たただけでBoolean型であるかどうかがわかること
- is + 形容詞　や　has + 過去分詞　が使われることが多い

参考
booleanメソッドの命名規則

三項演算子に書き換えよう

👎Bad

if (imageSize) {
  return '画像の容量が大きすぎるため、保存できませんでした。'
 }
 return ''

👍Good

return imageSize
  ? '画像の容量が大きすぎるため、保存できませんでした。'
  : ''

👉Point

• 三項演算子を適切に使用すると理解しやすくなる
• if文を簡潔に記述できる(上記例だとreturn を2回書く必要がなくなる！)

参考
条件 (三項) 演算子

v-forのkeyには一意の値を使用する

👎Bad

<list
    v-for="(campaign, index) in campaignItems"
    :key="index"
/>

👍Good

<list
    v-for="(campaign, index) in campaignItems"
    :key="campaign.id"
/>

👉Point

• 追加や削除など並び替えが発生する場合は、indexをkeyに使うと予期せぬ不具合に繋がる
• indexではなく一意の値を使用する

参考
Vue.js公式 #キー付き-v-for-必須
Vue.js: v-forで項目インデックスをkey属性にしていいのか

0かどうかの判定をする時は省略して書かない

👎Bad

<component v-if="!numOfItems" />

👍Good

<component v-if="numOfItems === 0" />

👉Point

• 数値の判定かどうかを式を見ただけで明確にわかるようにする
• 変数名からNumber型か分からない場合、確認する手間が増えてしまうため省略せずに 変数名 === 0 と書く

参考
[JavaScript] null とか undefined とか 0 とか 空文字('') とか false とかの判定について

undefinedとnullだけを弾く時の判定方法

👎Bad

if (item !== undefined && item !== null) {
    // 略
}

👍Good

if (item != null) {
    // 略
}

👉Point

• undefined と null だけの場合は上記のように完結に書ける
• 0 や false は弾かれない

参考
[JavaScript] null とか undefined とか 0 とか 空文字('') とか false とかの判定について

マジックナンバーを避けよう

👎Bad

if ([1, 2].includes(statusId)) {
    // 略
}

👍Good

// status.js
export const STATUS = Object.freeze({
  HAPPY: 1,
  SMAILE: 2,
  SAD: 3
})

// index.js
import { STATUS } from '~/status.js'

if ([STATUS.HAPPY, STATUS.SMAILE].includes(statusId)) {
    // 略
}

👉Point

• 番号だけでは何の判定をしているのかすぐに分からない
• 別ファイルに定数を定義することで明示的にわかりやすくする

まとめ

こちらは今までコードレビューを受けたものの一部ですが、また今後も増やして行きたいと思います。
この中でも私の解釈が間違えていたりもっと良い方法があったり、さらにレビューがある場合は優しく！！教えてコメントいただけますと幸いです。

そしてコードレビューで指摘されまくって落ち込んでいるそこのあなた！とてもわかります！

でもそれはまだまだ伸び代があるってことなので、心の中のフワちゃんに
「いや待って！アタシ伸び代しかないじゃーん！💘まじ最高！！✨✨」
と言わせましょう！！！