Help us understand the problem. What is going on with this article?

エンジニア3年目で初めてレビューをするようになったので最初にしたこと

More than 1 year has passed since last update.

この記事はラクス Advent Calendar 2018の23日目の記事です。
昨日は @aizoさんの 通信についてちょっと詳しく(物理寄り)でした!
本日はソースコードがない記事ですが、自分が初めて実施したことについての考察等をまとめてみました!


はじめに

この記事を書こうと思ったきっかけは、Webアプリエンジニアとして初めて「レビューしてね」
と言われたときに、「レビューってどういう観点で、どういうコメントをすればいいんだろう?」
となったので、そのときに実践したことを書きます!

導入

うちの現場では上長にソースコードや結合テスト仕様書をレビュー依頼する前に、
開発者同士で相互にレビューを行っています。これはスキルセットに関わらず、
該当のソースを改修する人はもれなく対象(レビュアーとしてもレビュイーとしても)です!
上長に見てもらう前に開発者同士でレビューし合うことによって、以下のようなメリットが考えられます!

  1. 次工程へ進むコードの質が高められる
  2. 他の人の書いたコードを読むことで、新しい知見を得られる
  3. 上長の工数を減らせるかもしれない

メリット

1.次工程へ進むコードの質が高められる

上長にレビューをしてもらう前でも、以下のような内容は開発者同士でも確認することができます。

  • 仕様に対するロジックがおかしくないか
  • 冗長な書き方をしていないか
  • わかりずらい命名をしていないか
  • (リファクタリングしているのであれば)修正前後でロジックに変化がないか
  • 変数やメソッドなど、共通化できる部分はないか
  • シンタックスエラー

2.他の人の書いたコードを読むことで得られるものがある

これはスキルセットが似通った開発者であっても、そうでなくてもメリットがあります。

スキルセットが同じ場合

  • 自分では書かないようなコーディング方法、ロジック、命名などがたくさん見れる
  • 「こう書いたらもっと良いのでは」などのような議論がソースコードを使ってできる

スキルセットが異なる場合

  • 自分では書けないコードを読む機会がもらえる (新しい発想・共通化の仕方・知らなかったライブラリ等)
  • 先輩が後輩にソースコードを見てアドバイスする経験を得られる

3.上長の工数を減らせるかもしれない

上長は忙しい上に、時間当たりの価値が自分より高い場合が多いです。
また、上記のようなことをしておくことで開発者同士でのノウハウが蓄積され、
同じ指摘を防ぐことなどもできます。

「レビューしてね」と言われて、まず何をしたか

現場でレビューをしてみようと言われ、実際に取った行動をまとめてみたいと思います。

1.マージリクエストの上長のコメントをすべて見る

これは主に以下の理由から、追える限りのすべてのコメントを追いました。

  • 影響範囲の大きなソースがPHPで書かれており、自分があまりPHPに精通していない
  • 「レビュー = 指摘」のような立場で実施したくなかったので、どんな言い回しをしているのか
  • コメントの種類として、どんなものが今まで多い傾向にあるか(命名・インデント・ロジックの不備・エラーハンドリング…等々)

ざざざっと見ただけでも、以下のようなコメントがありました。例として簡略化して箇条書きにしてみます。

・比較演算子は、厳密にする('==' → '===')
・正規表現が、何を許容したいのかをコメントに書いて欲しい
・$tmp(変数)は形式チェックをしてから使用しましょう
・この関数の第二引数は保証されていないので、明示的に値を渡してください

などなど、PHPのソースを読みなれていない身としてはいろいろと勉強になり、自分が見るときにも意識するきっかけになりました!

2.参考サイトを見て自分のなりのチェックシートを作る

以下のような参考サイトを見つつ、自分なりの手順をメモしてチェックシート代わりに使用していました。以下が参考にしたサイトです。

参考サイト

3."読みやすさ"とはなんなのかをもう一度意識する

レビューをするにあたって『リーダブルコード』を読み直しました。
久々に読み返しましたが、基本的な大事なことが書かれているので、年に一回、
半年に一回読み直そうと思ういい機会になったと思います。
※こちらの記事も"読みやすさ"についてわかりやすく書いていらっしゃいます。
リーダブルコードを3年ぶりに読み返してみて見つけた たった1つの大原則


まとめ

今の現場では、開発者同士でソースコードを見てレビューする、という工程を設けてもらったことで、
レビュー以外の時でも「なぜそう思うのか」を論理付けできるようになったり、違う視点からの考え方を
取り入れられるようになったりと、エンジニアとしてもとても良いスキルアップになったと思います!

シンタックスやロジックのチェックなどはIDEや静的コード解析ツールでもできる部分があると思いますが、
必ずしもそのようなツールがあるわけではないので、ご紹介させていただきました。

みなさんも、エンジニアとして初めてレビューをしてね!と言われたときなど、参考になれば
と思います!


明日の投稿はEichiSandenさんです!

Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
Comments
No comments
Sign up for free and join this conversation.
If you already have a Qiita account
Why do not you register as a user and use Qiita more conveniently?
You need to log in to use this function. Qiita can be used more conveniently after logging in.
You seem to be reading articles frequently this month. Qiita can be used more conveniently after logging in.
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
ユーザーは見つかりませんでした