実務未経験でインターンに参画してレビューでボコボコにされた話

はじめに

この記事は WESEEK Advent Calendar 2020 22日目の記事です。

2020年の7月から株式会社WESEEKでインターンをしている普通の大学４年生です。インターンではGROWI.cloudという SaaS のサービスを開発しています。

インターンを始めるまでは基本的に一人で勉強をしていたため、誰かにコードを見られるということがありませんでした。インターンを始めてからコードレビューでボコボコにされ（優しく教えてくれた）、独学ではなかなか得られない知識を頂戴したのでこの知識をシェアハビしたいと思いこの記事を書きました。

この記事では言語やフレームワーク問わず汎用的に実践できる点について、超初級と初級の二部構成でレビューで指摘をしてもらったことをまとめていきたいと思います。コードで説明をする部分に関しては javascript を使って説明してますが、難しいことは書いていないので javascript を触ったことがない方でも理解できる内容になっていると思います。

超初級

本当に入りたてのときはコードを書いてやっと動いたことに完全に安心し切ってコード以前のことをやってしまっていました。（今でもたまにやってしまう。。）

• タスクにアサイン忘れない
• コードに関係ないコメントは消す
• 前回のレビューで指摘されたことを修正し忘れない
• ci 通ってるか確認してからレビューを依頼する
  • 現場ではリモートリポジトリにプッシュをすると自動テストが走ることがほとんどだと思うので、落ち着いてテストが走り終わり成功するのを待ってからレビュー依頼をしましょう。
• マジックナンバーやマジックワードを使わない
• キャメルケースやスネークケースに注意する
  • 基本的に変数名やメソッド名はキャメルケースで書きます。

単純ですが意外と忘れがちなので気を付けていきたいですね！

初級

リーダブルコード的な観点でいうと、大きく分けて、冗長でないか、分かりづらくないかの二点に分けることが出来ます。最初は大量にプルリクにコメントが来てしんどかったですが、今となっては短く分かりやすいコードを書くことに喜びを感じています。

余分な変数定義

例えば、以下のような書き方は冗長です。予めjavascriptに用意されたメソッドを使うことによって、もっと可読性高く、かつ短く書くことができます。

冗長な例

const members = [{name:'kenta',age:21},{name:'naomi',age:15},{name:'shigeru',age:34}]
const adultMembers = [];

people.forEach( member => {
  if ( member.age >== 20) {
    adultMembers.push(member);
  }
});

上のコードは、以下のように書き換えることができます。

いい例

const members = [{name:'kenta',age:21},{name:'naomi',age:15},{name:'shigeru',age:34}]

const adultMembers = members.filter( => {
  return member.age >== 20;
});

これで数行コードを短く書くことができました！さらに filter メソッドを使うとコードに条件に当てはまる要素を抽出する処理をするというメッセージが加わり可読性が高まるという意味でもいいですね。

他にも、mapやreduceを積極的に使うことによってより良いコードを書くことができます。forEachは最終手段だと思っています。

SQL や API 叩きすぎ

プロジェクトではSQLやAPIをたくさん実行します。SQLクエリを叩いてDBへの問い合わせが増えれば増えるほど、APIを叩けば叩くほど、処理は重たくなっていきます。このような処理はなるべく数が少なくなるようにしたいです。

前提条件

• Organization と User は 1:多の関係

処理の流れ

冗長な例

アクティブな組織を全て取ってくる(SQLクエリ一回目)

条件に合致する組織を抽出

抽出した組織のユーザを全て取得(SQLクエリ二回目)

以下のような流れに変えることでSQLクエリの回数を減らすことができます。

良い例

組織を紐づくユーザとも含めて全て取ってくる(SQLクエリ一回目)

条件に合致する組織を抽出

抽出した組織のユーザを全て取得(SQLクエリ叩かないでmapメソッドなどで取ってこれる)

今回は簡単な例でSQLの一回くらい多くてもそんなに変わらないんじゃないかと思うかもしれません。しかし、このような一回一回の処理を軽くすることがプロジェクト全体でみたときにはとても大きな影響を及ぼすのでなるべくクエリの回数を減らしていきましょう。

同じような処理何度も書きすぎ

　違うファイルで何度も同じような処理を書いてしまっている時は util フォルダなどを自分で作成して共通のメソッドを作成しましょう。プロジェクト全体のコード量が減り、可読性が上がっていきます。

自分で頑張ってロジックを書いたけど実は他のメンバーがすでに util にメソッドを用意してくれて頑張って作ったやつが要らなくなってしまうことが何回かありました。自分でメソッドを書く前に既にないかを確認するべきですね。

if else じゃなくて三項演算子で良くない？

条件分岐についてです。

冗長な例

let sentense;

if(age >== 20){
    sentense = 'お酒が飲めます';
}
else{
    sentense = 'お酒が飲めません';
}

if else での条件分岐は以下のように参考演算子で書くことができます。

良い例

const sentense = age >= 20 ? 'お酒が飲めます' : 'お酒が飲めません';

これで一行で書くことができました。ただ、三項演算子の中の三項演算子のような所謂三項演算子のネストになってしまうと、確かに短くかけるものの可読性が著しく下がってしまうので使わない方がいいです。可読性と分かりやすさのバランスを考えながらコードを書いていきたいですね。

javascriptでは三項演算子の他にも条件分岐をすっきりかける手法がたくさんあるのでどんどん取り入れていきましょう。

その中でも論理演算子は使うことが多いです。

参考
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Operators/Logical_Operators
https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator

以下のコードは、

冗長な例

let databaseUrl

if(process.env.MONGO_URL){
    databaseUrl = process.env.MONGO_URL;
}
else{
    databaseUrl = 'https://databaseurl';
}

次のようにすっきり書くことができます。

良い例

const databaseUrl = process.env.MONGO_URL || 'mongodb://mongo:27017/'

一見datavaseUrlは真偽値が入ってきそうですが、実際には process.env.MONGO_URL がある場合は、その値を返し、なければ、'mongodb://mongo:27017/'を返すというロジックになっています。便利ですよね！

他にも、&& や ?? を使うことによってすっきりしたコードを書くことができるので是非使ってみてください。

条件式に否定の意味合いを入れるのをやめよう

以下のように条件式の中に否定をいれてしまうと読み手にとって非常に分かりづらいコードになってしまいます。

分かりづらい例

if(!flag){
    console.log('条件１')
}else{
    console.log('条件2')
}

以下のように書き換えた方が可読性の高いコードになります。

良い例

if(flag){
    console.log('条件2')
}else{
    console.log('条件１')
}

どうしても分かりづらい記述にはコメントをしよう

　変数名やメソッド名をわかりやすくしたりして分かりやすいコードを書いても、小難しいロジックを書くときなどは、どうしても分かりづらくなってしまいます。そんな時は積極的にコメントを書くようにします。書かないよりは書いた方が絶対いいと思う（コメントを書きすぎて指摘されたことはほとんどない）ので積極的にコメントをコードに残していきましょう。

まとめ

　以上に挙げた点意外にもたくさんレビューで指摘をしてもらい、自分で勉強しているだけでは絶対に気づけないようなことも教えていただいているので現場で揉まれることの重要性を日々感じています。

　そして、レビューとは関係ないですが、やっぱりチームで何かを作って実際にサービスが誰かに使ってもらえるってめちゃくちゃ楽しいです。これからも楽しみつつ頑張っていきたいと思います。

※この記事は WESEEK Tips wiki に 2020/12/22 に投稿された記事の転載です。
Tips wiki では、IT企業の技術的な情報やプロジェクトの情報を公開可能な範囲で公開してます。

参考にした資料