jfluteの日記 このページをアンテナに追加

2016-09-12

レビューしやすいコード (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の空中戦になりやすいからですね。
とはいえ、やっぱり言葉しては必要で使うは使いますが...
"レビューしやすいコードのための表現方法の一つ"
というニュアンスです。

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

余談: 差分レビューの罠

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

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

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

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

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

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

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

...

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

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

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

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

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

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

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

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

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

...

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

もともとの極論として、

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

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

ズバリ言うと...

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

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

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

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

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

...

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

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

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

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

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

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

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

ことでしょう。

考え続けることが答え

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

...

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

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

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

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

...

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

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

f:id:jflute:20100906184928j:image