レビューしやすいコード (Reviewable Code)

れびゅーわぶる!?

レビューしやすいコード

よく、jfluteが現場で掲げるコーディングテーマです。主に、事業会社でのサービス開発における、スタートアップの時期から少し経ってチーム開発し始めた、インクリメンタル開発が続いてきた時に適用します。

色々とたくさんは言わず、とにかくこれだけは心がけよう、とディベロッパーにお願いしていることです。

インクリメンタル開発に大切なこと

インクリメンタル開発の現場をデザインする上で、大切にすべきことが大きく二つあります。

ビジネス貢献の向上

もうさすがに言わずと知れたことですが、事業会社ではビジネスとプログラミングが非常に密接です。ディベロッパーもアーキテクトもビジネス貢献を忘れて、的外れなものを作っても自分が苦しむだけです。

スタートアップ&インクリメンタル開発の特徴がよくわからない方は、こちらの資料がオススメです。
 => LastaFlute First Impact | Speaker Deck

ビジネスと言うと少し大げさに思われる方は、そこを "問題解決" と置き換えてもいいでしょう。

ビジネスを成功させるためにどうしたらいいか?

ディベロッパーが対峙している課題は、そこから距離はちょっと遠いものではありますが、ぼくらはそこに頭をひねって、間接的でつながりを考える必要があります。

(本来、ここだけでたくさんのお話があるのですが、今日の焦点はここではないので割愛します。このビジネス貢献の向上を下支えする要因は何か?そこに焦点を当てていきます)

プログラミング環境の向上

事業会社での技術課題は非常に多様的です。エンジニアの技術追求の意識も求められます。優秀なエンジニアに来てもらうことも大切ですし、優秀なエンジニアに長くいてもらうことも大切です。(どうしても人の入れ替わりは激しいものです)

いくらビジネスへの意識を高く持ってもらったとしても...

根本のプログラミング環境が乱れていると、ビジネスへの意識をキープできないのです

これは、マズローの欲求と似ているかもしれません。高次の欲求は、低次の欲求の上に成り立っています。"本質的でないことに時間をかかるような現場" だと、モチベーションは下がります。そして、高次の欲求までたどり着けないのです。
 => エンジニアのモチベーションを下げる方法 | jfluteの日記

単純に、ディスプレイや椅子などのわかりやすいものから、開発環境の充実、そして、コード上でのスムーズな立ち回り、これらがポイントになってきます。

ただでさえ人の入れ替わりの激しい現場、そして特にいま (2016/9現在) の売り手市場、高い技術課題を解決できる人に現場にいてもらい、そして、その人に高いビジネス意識を持ってもらい、有益な成果を出してもらうためには、低次の欲求をしっかり満たすことが前提です。

そう、

ビジネス貢献とプログラミング環境の向上は、密接につながっている

のです。

コードの乱れが生むもの

さて、コードが乱れていると、
何が起きる?

デメリットの連鎖

インクリメンタル開発は、非常にコードの乱れが発生しやすい現場です。そして、その乱れのデメリットをもろに食らいやすい現場です。人の入れ替わりがそれを発生させるし、それが人の入れ替わりを発生させます。

短期的: 長いレビュー、バグの見逃し、緊急リリース、手戻り
中期的: 長い既存コード分析、メンテナンスモチベーション低下
長期的: システム再構築or非効率メンテ、優秀エンジニアの離散

短期的デメリットの対応を誤ると、中期的デメリットが顕在化してきます。
中期的デメリットは対応しづらく、積み重ねると長期的デメリットになります。

短期的デメリットのストーリー

読むのもつらい把握しづらいコード、レビューは長くなり、レビューワーの負荷も高くなり、バグは見逃し、緊急リリース...

スタートアップ直後は、ビジネス規模もシステムの規模も小さく、緊急リリースのコストは低いですが、インクリメンタルで積み重なってくると、気軽にリリースも少しずつできなくなってきます。ちょっとした修正でも、緊急リリースに奪われる時間が増えてきます。

また、レビューでの見逃しが発生すると、リリースしてから、「実はそういう実装じゃないほうがよかった!」と判明して手戻りコストが発生したりもします。

中期的デメリットのストーリー

そして、様々な負のパターンも生み出し...
 => メンテ不能の強者、引数リモコンパターン | jfluteの日記
 => ルーズなDaoパターンなら見たくない | jfluteの日記

気づいたら、ちょっとしたこともすごく時間がかかるようになって、なんだか働いてるわりには先に進んだ感が出ない...「なんか、この現場のコード直すお仕事、いやだなぁ...」メンテナンス意欲が下がり、別の仕事したくなってきます。

一方で、それに明確に気づかないのも問題です。ある日突然足が重くなるわけじゃなく、徐々に徐々にそうなっていくので慣れてしまうのです。一年前と正確に比べるのは難しいことです。効率悪いのに効率悪いと思わないでずっと...

人もどんどん入れ替わっていくので、初めて来た人はなおさらわからない。それに気づくのは優秀なエンジニアだけ。でも解決は困難。チームの問題意識がバラバラだから。

長期的デメリットのストーリー

最悪の長期戦へ。

「これ作り直そうよ!」しまいには、システムの再構築という、大きな代償を会社が払うことになります。でも、そんなコストを払えるか?そのためにビジネスを止められるか?

なかなかできません。そこで続くのは非効率メンテ、しかも長期化。優秀なエンジニアは敏感なので、どんどん立ち去っていきます。様々な理由を見つけて現場からいなくなります。

すると逆に、「やっぱり、再構築やろう!」と英断したところで、現場にスキルが足りず、再構築プロジェクトを進めることもできません。

両方だいじなんです

一方で、例えば "レビューが長くなる" からと言ってレビューせずにリリースをすると、早くリリースできた!と思って短期的に喜んでいたら、実は中期的デメリットを生み出すことになります。

しかしながら、コードの乱れでレビューが長くなることによって、レビューワーのレビューに対する心理的重荷が発生します。なので、自然とレビューに対して非消極的な対策をする空気になっていきます。これは一つの罠と言えるでしょう。

安易に逆サイドに振れる対策を取ると、行ったり来たりを繰り返すことになります。

なぜなら、"両方だいじ" なのですから。
 => 両方だいじなんです | jfluteの日記

バランスを見つける旅だし、両立を見つける旅なのです。

レビューがしやすければ?

逆に、レビューがしやすければ...

o レビュー時間の短縮
  => 早いリリース
   => ビジネス貢献

o レビューでバグ発見
  => 品質の向上、緊急リリース削減
   => ビジネス貢献

o 読みやすいコードの積み重ね
  => 既存コード分析時間の短縮
   => メンテナンスモチベーションのキープ
    => ビジネス意識のキープ
     => ビジネス貢献への積極的な行動

o 変ななコードもレビューで発見
  => 経験浅いディベロッパーのコードもリリースできる
   => リソースマネジメントがしやすくなる
    => ビジネス貢献

o (...などなど)

つまり、先ほどのコードの乱れのデメリットを抑える効果があり、そして...

レビューがしやすいコードというのは、非常にビジネス意識の高いコード

と言えるでしょう。

"プログラミング環境の向上" の一つであり、非効率メンテの長期化によるエンジニアの離散を防ぎ、エンジニアのビジネス意識に下支えする重要な基盤となり、結果的に" ビジネス貢献の向上" につながります。

...

また、まだ経験の浅いディベロッパーであっても、とにかくレビューしやすいコードさえ心がけていれば、何かへんてこりんでも優秀な先輩たちが指摘してくれます。バグを見つけてくれます。それで問題は未然に防げます。それでビジネスを回していくことができるのです。

また、フィードバックをたくさんもらえることで、スキルアップのチャンスにもなり、指摘も少なくなってきて、早いリリースができるようになっていきます。レビューさえしやすければ、いきなり正解を引かなくてもいいわけです。

だから、まずはそこ。

レビューしやすいコードとは?

大きくふたつ:

o 統一的なコード
o 理路整然としたコード

統一的なコード

一番簡単に思いつくものはコードの統一性です。もちろんこれがすべてではありませんが、大きな要素であることは確実です。

そのための工夫も様々です。仕組みや運用での工夫が大きな地盤になるでしょう。

_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/
o 信頼できるアーキテクトにアーキテクチャデザインを依頼
 => アーキテクトのちから | jfluteの日記

o フレームワークをしっかりと使いこなす
 => フレームワークの規約やポリシーに乗っかる

o IDE警告、checkstyleなどによるコーディングスタイルの統一
o ハンズオン研修でのスキル向上と兼ねてポリシーの意識統一
o レビューによる非定型なパターンのコード改善
o (...などなど)
_/_/_/_/_/_/_/_/_/_/

ダメなコード、ダメなポリシーで統一してもダメですから、"アーキテクチャデザイン" は重要です。アーキテクトのスキルや気づかい力に依存します。

一方で、ディベロッパー本人の意識やスキルにも依存します。ここも "両方だいじなんです" ですね。仕組みと属人の両サイドから攻めていって、初めてうまくいくものと言っていいと思います。そう簡単なことではありません。

理路整然としたコード

一方で、なんでもかんでも統一すればいいわけではありません。ときには問題解決のために部分最適をする必要もありますから。

どうしても統一できないところがあったとしても、筋道が通っており、かつ、シンプルに表現されていれば、メンテナンスする上で困らないコードになります。つまり、"理路整然としているコード" です。そういうコードが書けるプログラマーは確実に存在します。

ここが世でよく言われる "綺麗なコード" とか、"わかりやすいコード" という言葉に相当するかもですね。

...

安心してください。そこまで敷居の高い話ではありません。

o そのロジックに行き着いた背景をコメントする
o 目的と手段を分離したコードを心がける
o などなど

これらをちょっと意識するだけでも、理路整然に辿り着くための大きな第一歩になります。お手軽なのでそこから実践してみるとよいでしょう。

...

ちなみに、綺麗なコードという表現は、誤解を生みやすいのであまり使わないようにしています。必要不要の1/0の空中戦になりやすいからですね。とはいえ、やっぱり言葉しては抽象表現として必要で、使うは使いますが...jfluteとしては大抵の場合 "レビューしやすいコードのための表現方法の一つ" というニュアンスですね。

あくまで、"どういうコードがビジネスに貢献するのか?" そこが論点だからです。お金をもらっているからには。

余談: 差分レビューの罠

レビューはとてもよい習慣で、レビューによるコードの向上の効果は非常に高いものです。また、レビューをすることで、他の人のコードを読むきっかけになりますし、コードで人とコミュニケーションするきっかけになります。経験の浅いプログラマーの教育にも非常に効果的です。

最近は、Githubのプルリクを使ってレビューが増えてきました。差分が非常に見やすく表示され、コメントもしやすくなっています。ただ、ちょっと差分レビューの罠っぽいレビューをよく見かけます。

差分だけ見て全体をよく見渡すことなく、プルリクのコメントだけで空中戦をしてしまうこと。

ついつい、Githubのプルリクの画面だけが、レビューの場になってしまいがちです。周りのコードがこうなっているからこそ、そう修正されたのであって、差分のコードだけではなんとも言えなかったり。

クラス構造レベルの大きな修正をしたときは、差分のコードだけではよくわかりません。

「このチケットは face-to-face でやりましょう」とか、運用として何かアドリブがあって、レビュー会でレビューイーが説明できるとよいでしょう。

リモートのままだとしても、レビューイーが概念的な説明を書いて、レビューワーに全体像を伝えられるとよいでしょう。

...

また、Githubのプルリク画面だけでレビューが終了し、IDE警告やcheckstyleの見逃し防止がされないのも、よくあるアンチパターンです。

その基本的なレベルの警告コードがプルリクに上がってくると、レビューが長くなって仕方ありませんし、バグ発見や業務的なミス発見のためのレビューのノイズになり、本質的なレビューに影響が出ます。まさしく短期的なデメリットです。かといって見逃せば、中期的デメリットを積み重ねます。

これはちょっと性悪説ですが、ディベロッパーの入れ替わりが激しいと、どうしてもIDE警告やcheckstyleは見過ごされがちです。気づいたら警告だらけ (真っ黄色) になって、そうなると新しく発生した警告も気にしなくなります。事実上の無意味チェックです。

レビューワーの誰か一人でも、IDE上でコードを開いてチェックする人を設けたいところです。

余談: 自由と法の絡み合い

ツールによる統一的なチェックは運用が難しいものです。

先ほどもありましたが、常に統一というのは難しく、ケースバイケースが必ず存在するからです。

また、あまりに厳しくなると窮屈にも思えてきて、別のモチベーションダウンも発生します。プログラマーとしてある程度は個性も出したい、と思うのが心情です。(もちろん、理路整然としていること大前提で)

どこまでチェックするか?このバランスは常に様子を見て微調整し続けることが大切です。法律も時代が変われば変わってきます。

...

さて、運用していけばいくほど、チェックがどんどん厳しくなっていきがちです。
なぜ?

もともとの極論として、

"チェックしなくても統一的で理路整然としたコードが積み上がっていくならチェックする必要ない"

と言えますが、でも実際はなかなかそうなりません。でもレビュー会でそのレベルのことまでチェックしていたら、レビューの時間がながくなってしょうがない。なので、ツールによる自動化をするわけです。

ズバリ言うと...

ディベロッパーの質が低ければ低いほど、どんどん厳しくなっていく

のです。(厳密には、そういうディベロッパーが混じると)

でもツールは、ケースバイケースやバランスを知りません。融通の利かない画一的なチェックしかできないのです。

もし将来、人工知能checkstyleが登場したら、「まあ、ここはわかりやすいからOKだぜ、いえーい」とケースバイケースまで考慮してくれるかもしれませんが、それ、いまはないです。そこでギャップが生まれます。

これは世の中がそうなっています。以前は禁止されていなかったものが禁止されたりします。安易な不注意で事故を起こす人や悪用したりする人がいるから、禁止になるわけですが...注意深くしていて事故も起こさず悪用もしない人からすれば、「えー、べつにいいじゃん」って思うこともあるでしょう。

...

自由や個性を得たければ、ディベロッパーとして...いや、プログラマーとして、

最低限の統一的を保ちながらも理路整然で個性も入ってるコードを書けるようにトレーニングし続ける

のが、ぼくらの宿命でしょう。

レビューワーへの気づかい

レビューしやすいコードを書くためには?

先ほど、統一的なコードのお話のところで、いくつかの工夫は出しましたが、極論は...

o 現場現場で考えていかないといけない
o 個人個人で考えていかないといけない

ことでしょう。

考え続けることが答え

と言えるかもしれません。

...

一方で、それをトレーニングすることは幾らでもできます。

o レビューワーにたくさんなればいい
o ソースコードをたくさん読めばいい
o 書いたコードを人に説明すればいい
o リーダブルコード買ってくればいい

よくエンジニアは、「このときはこうすればOK、このときはああすればOK」というルールを求めがちですが、この世の中、そんな簡単な世界じゃない。

お店のスタッフがどういう接客すればお客様が喜ぶか?...お客様視点をまったく持っていないスタッフが、「このときはこうすればOK、このときはああすればOK」ってマニュアルが欲しいと言ってきたらどうしますか?

...

レビューワーへの気づかい

そこから思考の始まりです。