よもやま話β版

よもやま話を書きます。内容はぺらぺら。自由に書く。

すばやく確認OKをもらうためのセルフレビュー

こんにちは、@beta_chelsea です。 ご縁ありまして、今年の秋頃からフィヨルドブートキャンプにメンターとして参加させていただいております。ということで、フィヨルドブートキャンプ Part 2 Advent Calendar 2020 - Adventarの記事を書きます。

昨日は メンター @u1tnk さんの 「失なわれた構造化プログラミング、そしてオブジェクト指向へ… 」、構造化プログラミングのお話でした。 ソースが実際に構造化されていく様子、実際に見ないとピンとこないことも多いかと思います。特にオブジェクト指向にお悩みの方はぜひご一読ください!

自分は「セルフレビュー 」をテーマにちょっと述べてみたいと思います。 フィヨルドブートキャンプは、メンターが生徒さんたちの書いた成果物(ソースコード)に「確認OK」を出すことで進めていくのですが、セルフレビューを活用することでそのサイクルがすばやくなるシーンもありそうだなと考えています。

TL;DR

「とにかく見直ししましょう!」ということを延々と述べます。

ルフレビュー is 何

「自分自身のアウトプットを自分自身でレビューをすること」を指して「セルフレビュー 」と呼んでいます。 なんとなく使っていた単語なのですが、一応Google先生に尋ねてみると自分と同じ解釈で使っている人もいるようで安心しました。試験にも出たことがあるようです。

基本情報技術者過去問題 平成25年春期 午後問6(プロジェクトマネジメント)|基本情報技術者試験.com

プログラミング担当者が単独で行うのがセルフレビューであり,プログラミング担当者ともう1名でペアを組んで行うのがペアレビューである。セルフレビューの終了後にペアレビューを行う。

自分以外の誰かにレビューをお願いする前のセルフレビューがうまく機能すると、ちょっとしたことで差し戻しを受けてしまうということがグッと減ると思います。また、レビューの内容としても、より本質的な議論に集中した濃いものになるでしょう。 フィヨルドブートキャンプでよく行われている「GitHubのPull Requestの機能を使ってレビューを受ける」シーンを考えつつ、セルフレビューのポイントを挙げてみたいと思います。

見直しする

Pull Requestを作ると、今回どんな変更を行ったのかをFiles changedのタブから確認することができます。

GitHubのPullRequestページのFiles changedのタブ
Files changedのタブ

レビューする人(以下「レビュアー」、フィヨルドブートキャンプではメンターが担当します)が主にチェックするのはこのFiles changedのタブの中身となります。この変更内容を、レビュアーにチェックを頼む前に自分自身で一通り眺めてみましょう。 結構、次のような気づきがあるケースが多いかなと思います。

  • Pull Requestの目的に関係のない変更が混じっていた
  • 意図しない半角スペースが混じっていた
  • typoがあった

作成者自身が見て気づくことは、そのままレビューをお願いすれば差し戻される要因になってきます。 レビュー依頼前にFiles changedを自分自身でも見直し、修正するクセをつけておくと、将来の自分の修正の手間も減らすことにつながります。

必要なら事前コメントを入れる

Pull Requestを見直していて「これはレビュアーに何かつつかれそうだけど、〇〇という事情があって、この実装になってしまった。仕方なかったんだよなぁ」と思うことが避けられない場合もあるでしょう。そんな時は、コメントを利用し「なぜこの実装をしたのか?」という事情を先んじて補足しておくのも一手です。先に補足があると、レビュアーからの「ここを〜〜のように書かなかったのは何故ですか?」という質問に答える手間が省略できて、さらにもっと重要な議論に早く移れるという利点もあります。

さらに見直しする(再レビュー依頼前)

例えば、「この変数はheartよりもstarという命名が適切です」というレビューがとあるファイルの10行目にコメントされたとします。10行目のheartという変数名をstarに修正し、改めて再レビューの依頼をした結果として、別のファイルの32行目にまったく同じレビューコメントをもらってしまった...みなさん一度はそういう経験がありませんか? こういったちょっと残念な事態の防止には、レビュアーから一度貰ったコメントが他の箇所にも適用されないかの「見直しセルフレビュー 」が効果的です。レビュアーは人間なので、Lintツールのように類似問題箇所を完璧に洗い出して各1行1行にコメントをつける...というのは残念ながら難しいでしょう。レビュアーの見落としを発見するくらいの気持ちで、改めてセルフレビューをやってみてください。他の箇所も同様に修正対応すべきか否か迷うケースの場合は、レビュアーと相談してみましょう。

翌日見直しもおすすめ

Pull Requestを作ってレビューを依頼しようと思ったということは、「完成した!」と一度は考えたということでしょう。「完成した!」という気持ちを持っている状態から、セルフレビューで自分自身が見落としていた問題を看破することはなかなか難しいことです。ちょっと変更内容がややこしいものだったり、モヤっとした予感があるけどピンとこない、といった時は、あえてその日は寝かせておいて翌日の自分にレビューしてもらうという方法も効果的かなと思います。一旦実装から離れてから改めて見直すことで、過去の自分の「やらかし」を看破できたというケースを体験した人も少なからずいるのではないでしょうか。 緊急時は残念ながら使えない手段ですが、自分は普段できるだけ当日見直しは避けるようにしています。

レビュアーとして気をつけていること

上記で紹介したことは自分も日頃の仕事のなかで注意し、実践していることです。一方で、フィヨルドブートキャンプでは「レビュアー 」として活動しています。生徒さんをお待たせしてしまわないためにも、一発で伝わるレビューになればと思い、次のことに気を付けるようにしています。

他の箇所も再度見直しして欲しい時は一言添える

上記の「同じ内容を再レビュー」の予防として、似た問題がPull Request内に散見されることに気付けた場合は、その旨を一言コメントに添えるようにしています。ただし、似た問題があること自体を見落としていた...というケースもあったりするので、やはり再レビュー依頼前の見直しはオススメしていきたいです!

相手と語彙や認識を揃えていく

「アンダースコア」「アンスコ」「アンダーバー」、これらの語句は全て同一の _ を指しています。 もしやりとりの中で先に相手が特定の単語を使っていた場合、同じものを指すときは同じ表記でコメントするようにしています。もしくは、相手が伝えようとしていることについて複数の解釈がある場合は、解釈AとBについて書き出してみて、どちらでしょうか? と尋ねることで相手の考え方と自分の認識を揃えていきます。 うっかり認識がずれたままレビューを進めると、不幸な勘違いコントのような状況が出来上がってしまい、それが発覚するのはソースコードの修正結果が出てきてから...ということも残念ながら時々あるケースです。できるだけその確率を減らすため、語彙や認識がちょっとでもズレそうな要因を排除するようにしています。

指示語は出来るだけ減らし、主語をハッキリさせる

「この引数は使われていない」という言い方よりも「引数valueは使われていない」と伝える...といったように、何を対象としているかをよりハッキリ示せるような書き方を心がけています。 こちらも「認識のズレ」を出来るだけ無くそうとする方向性の試みです。 いわゆる「こそあど言葉」は認識がズレやすい要因になりがちなので、気付いたら書き直すようにしています。

おわりに

さて、気を付けていることを書き出してみたものの、毎度完璧にできているかというと、うーん、なかなか難しいものです...。 また、フィヨルドブートキャンプのメンターとしてのレビューは、自分で何らかの答えを見つけた時が一番学びになるとも考え、あえてフワッとしたイジワルな言い方をしてしまうときもあります。すみません!

まだ学生の時分、プログラマーという職業の人たちは大きな企業の半個室ブースにこもって、あんまり人と会わなくてもいいし、とにかくプログラムと見つめあっていればいいんだ、コミュニケーション苦手な自分にとっては最高の職業だ...と思ってたことが実はありました。ところがどっこい蓋を開けてみると、これほど精緻なコミュニケーションを求められる職もないのでは、そう感じるシーンがあらゆるところに発生していました。コミュニケーションをおろそかにしたまま進むと、成果物が顧客の本当に求めていたものから180度逆の方向性に仕上がっていた...なんて事態になりかねません。 より良いものを作っていくため、言葉を尽くして自分の考えを伝えるテクニックをまだまだ磨いていきたいと思います。また、その重要性をフィヨルドブートキャンプの生徒さんたちにも感じてもらえるようなレビューが出来るよう頑張ります。

2021年も引き続きよろしくお願いします!