Node.js、TypeScript、jestでのテストコード、awsが絡む実装において、指摘事項を少しでも減らすためにやること

## 記事内容について
新卒の研修時代に受けたものを含めた指摘事項一覧です(覚えてるやつだけ)
そもそもコードレビューにおける指摘事項に関しては、開発チームの方針に基づくものなのでこの記事の内容をコードに反映させたらレビューが通るわけではないというのは当然の話ではありますが、少しでも参考になればいいなと思います。

過去に受けたことにある指摘事項で覚えているものをリストとして書きます。
またにチーム独特の思想とかも混じってる可能性もある

## 1.使ってない変数、関数を削除
➡そりゃそうやって感じかもしれませんが、でもコードの行数が多いと、使ってない変数に表示される黄色い線を見落とす可能性がある。
エディタによっては警告とエラー含めて一覧を表示してくれるが。。。

## 3.console.log()の消し忘れないようにする
➡開発の中でちょっとした値の確認とかでついつい使っちゃうものだが、これの消し忘れはあるあるではないでしょうか。
もちろん、開発の中ではちゃんとしたロガーを実装すると思うので、そのロガー以外でconsole.logの余計なログが混じってるのはよろしくないため、削除する。

## 2. var、letを使わず、変数は全てconstにする
詳しくは、https://qiita.com/kiyoshiro/items/13c60fad1f5279993fa2

実際、案外いける。ラムダ実装で、最後まで処理が成功するまで何度でもやり直す的な実装をしたときは、どうしてもletを使ったが、そんな感じの実装じゃなければ案外全部constでいけちゃうもんだった。

## 3.非同期ではforEach使わない。
forEachは非同期処理において待ってくれない。Promise.allで囲ってmapを使ってループさせる。

## 4.awsのラムダを実装する際、ハンドラーの中は最低限パラメータを受け取るぐらいにする
```typescript:anime-suko.ts
export const handler: APIGatewayProxyHandler = async (event) => {
const hogeId = event.requestContext.hogeId

  //ラムダ関数。この関数の中で具体的に実装する
  await hogeLambdaFunction(hogeId)

  return {
    statusCode: Number(HTTP_STATUS_OK),
    body: '',
  }
}
```

## 5.関数を作る際に「function」と書かずに、アロー関数で統一
2つの方法でできることが書き方混ざってるのが嫌ってのがでかいのとバインド多用するとコードが見づらくなる

## 6.dynamoのテストデータを生成する際、テストケース毎にput()ではなくbatchWrite()で一括生成する
テストケースで必要なテストデータが1件とかなら別にput()でもよいが、多い場合はbatchWrite()使って一括で生成する。
ちなみに、テストケースが多く、テストケースの中で1件ずつput()しているテストケースがいっぱいあったら、テストの処理が終わるのにめっちゃ時間かかるし、バグの原因になる。

## 7.コメントアウトする際、横に書いたり上に書いたりバラバラにせず、統一する

```typescript:anime-suko.ts
//いやっほ～
await otaku.hoge().promise()

await otaku.fuga().promise() //秋葉原は、わいの家
```
## 8.jestでのテストコード実装の際、基本的に変数はtest()の中
変数は、テストケースごとに管理させたいし、varを使わない気持ちに似てる

## 9.typescriptでの型指定による警告は必ず全て修正する
型があいまいで、anyとか使うと警告が出たりするが、ほったらかしにするのはだめ。
ちなみに、typescriptはjavascriptと違って、関数の引数やインタフェースの変数などに型を指定しなければならない。

```typescript:anime-suko.ts
const hogeFunction = async (name: string) => {
   //いろいろ
}
```

## 10.なんでもかんでもawaitつけない。
Nodeを勉強したばかりのとき、１つでもawaitをつける関数を作ると、他の関数も全てawaitつけて統一しなきゃいけないのかなと思って全部書いちゃってた。
awaitは、データベース取得や外部通信するみたいな比較的処理に時間がかかり待ってほしいときに使う。なんでもasyncな関数にしてはだめ。むやみに非同期にすると、意図しない処理の順番になったりする。
普通に変数同士で計算したり、文字列操作などをするみたいな関数には付ける必要がない。

## 11.空文字とかnullとかundefineを別々で判定しなくても一回でいける
**❌な例**

```typescript:anime-suko.ts
if(hoge == ''){
}
if(hoge == null){
}
if(hoge == undefined){
}
```

**🔴な例**
以下のように、ビックリマーク(論理否定演算子)をつけるだけで、上記の3つとも判定できます。

```typescript:anime-suko.ts
if(!hoge){
}
```

## 12.バリデーションの際、条件ごとにifいっぱい使わない
➡以下のように、returnの中で直接値を見て、trueかfalseを返せばいい

```typescript:anime-suko.ts
const checkValidation: (data_obj: {
  hogeNo?: string
  message?: string
  title?: string
  phoneNo?: string
}) => boolean = (data_obj) => {
  return !(
    (!data_obj.phoneNo && !data_obj.hogeNo) ||
    !data_obj.title ||
    !data_obj.message
  )
}
```

## 13.awaitとthenを一緒に使わない。
➡awaitは、asyncな関数に対してこれをつけて関数を呼ぶだけで待ってくれる
➡thenはPromiseを使うときに使う

## 14.テストコードを書くときは、4つのフェーズを考える
```
1.setup・・・DBの値を入れる、環境を設定する、mockの挙動を設定する、引数の準備
2.exercise・・・テスト対象のメソッドを実行する(jestでいうtestメソッド)
3.verify・・・検証(jestでいうexpectメソッド)
4.TearDown・・・後片付け(テストデータの削除したり、モックをリセットしたり)
```

## 15.インデントを忘れない
➡普通はエディタの自動整形機能を使うが、設定されてなくて自動整形してない場合もある。

## 16.プルリク全体としてのファイルの差分で、あまり赤い線が表示されないようにする。
コードレビューをする際、直接ファイルを見るのは大変なので、差分を見てレビューすると思います。
直接変えたところは赤くなったりするのは仕方ないが、コンフリクト修正とかで行がずれて
赤い線がいっぱい表示されるのはあまりよくない

## 17.直接文字列で名前を書いているところは全て環境変数にする
以下のように条件とか、ステータスコードとか、システム独自で使う何かしらの名前を直接コードの中に書くと、変更があった際にいろんなコードを見なければならない。エディタの全検索で一括置換もできるが、管理をする上では環境変数としてまとめるべき。

```typescript:anime-suko.ts
if(hogeName == "anime"){
}
```

```typescript:anime-suko.ts
const {
  HOGE_NAME_OTAKU,
} = process.env

if(hogeName == HOGE_NAME_OTAKU)
```

## 18.配列に要素が存在するかの処理で、indexOfではなくincludesを使う
●名前が直感的だし、返ってくる値も分かりやすい。
includes・・・あったら、true。無かったらfalse
indexOf・・・あったら、その場所の数値、無かったら-1が返る

●indexOfは、NaNと空の要素を見つけられないというデメリットもある。

これはもうincludes一択やな

## 誰しもが共通する、指摘事項の減らし方
指摘事項を受けるたびに、メモに追加する。
そしたら、次のタスクで同じ指摘を受けることがなく、段々と指摘事項が減っていく。
自分の場合、人生で初めてコードレビューを受けたときは、指摘事項10件以上あったっけな。。。(困惑)
そんな中、メモするようにしてからとある実装タスクで指摘事項無しで通ったときの嬉しさはやばかったです(小並感)