れびゅーわぶる!?
レビューしやすいコード
よく、jfluteが現場で掲げるコーディングテーマです。主に、事業会社でのサービス開発における、スタートアップの時期から少し経ってチーム開発し始めた、インクリメンタル開発が続いてきた時に適用します。
色々とたくさんは言わず、とにかくこれだけは心がけよう、とディベロッパーにお願いしていることです。
インクリメンタル開発に大切なこと
インクリメンタル開発の現場をデザインする上で、大切にすべきことが大きく二つあります。
ビジネス貢献の向上
もうさすがに言わずと知れたことですが、事業会社ではビジネスとプログラミングが非常に密接です。ディベロッパーもアーキテクトもビジネス貢献を忘れて、的外れなものを作っても自分が苦しむだけです。
スタートアップ＆インクリメンタル開発の特徴がよくわからない方は、こちらの資料がオススメです。
=> LastaFlute First Impact | Speaker Deck
ビジネスと言うと少し大げさに思われる方は、そこを "問題解決" と置き換えてもいいでしょう。
ビジネスを成功させるためにどうしたらいいか？
ディベロッパーが対峙している課題は、そこから距離はちょっと遠いものではありますが、ぼくらはそこに頭をひねって、間接的でつながりを考える必要があります。
(本来、ここだけでたくさんのお話があるのですが、今日の焦点はここではないので割愛します。このビジネス貢献の向上を下支えする要因は何か？そこに焦点を当てていきます)
プログラミング環境の向上
事業会社での技術課題は非常に多様的です。エンジニアの技術追求の意識も求められます。優秀なエンジニアに来てもらうことも大切ですし、優秀なエンジニアに長くいてもらうことも大切です。(どうしても人の入れ替わりは激しいものです)
いくらビジネスへの意識を高く持ってもらったとしても...
根本のプログラミング環境が乱れていると、ビジネスへの意識をキープできないのです
これは、マズローの欲求と似ているかもしれません。高次の欲求は、低次の欲求の上に成り立っています。"本質的でないことに時間をかかるような現場" だと、モチベーションは下がります。そして、高次の欲求までたどり着けないのです。
=> エンジニアのモチベーションを下げる方法 | jfluteの日記
単純に、ディスプレイや椅子などのわかりやすいものから、開発環境の充実、そして、コード上でのスムーズな立ち回り、これらがポイントになってきます。
ただでさえ人の入れ替わりの激しい現場、そして特にいま (2016/9現在) の売り手市場、高い技術課題を解決できる人に現場にいてもらい、そして、その人に高いビジネス意識を持ってもらい、有益な成果を出してもらうためには、低次の欲求をしっかり満たすことが前提です。
そう、
ビジネス貢献とプログラミング環境の向上は、密接につながっている
のです。
コードの乱れが生むもの
さて、コードが乱れていると、何が起きる？
デメリットの連鎖
インクリメンタル開発は、非常にコードの乱れが発生しやすい現場です。そして、その乱れのデメリットをもろに食らいやすい現場です。人の入れ替わりがそれを発生させるし、それが人の入れ替わりを発生させます。
短期的: 長いレビュー、バグの見逃し、緊急リリース、手戻り
中期的: 長い既存コード分析、メンテナンスモチベーション低下
長期的: システム再構築or非効率メンテ、優秀エンジニアの離散
短期的デメリットの対応を誤ると、中期的デメリットが顕在化してきます。
中期的デメリットは対応しづらく、積み重ねると長期的デメリットになります。
短期的デメリットのストーリー
読むのもつらい把握しづらいコード、レビューは長くなり、レビューワーの負荷も高くなり、バグは見逃し、緊急リリース...
スタートアップ直後は、ビジネス規模もシステムの規模も小さく、緊急リリースのコストは低いですが、インクリメンタルで積み重なってくると、気軽にリリースも少しずつできなくなってきます。ちょっとした修正でも、緊急リリースに奪われる時間が増えてきます。
また、レビューでの見逃しが発生すると、リリースしてから、「実はそういう実装じゃないほうがよかった！」と判明して手戻りコストが発生したりもします。
中期的デメリットのストーリー
そして、様々な負のパターンも生み出し...
=> メンテ不能の強者、引数リモコンパターン | jfluteの日記
=> ルーズなDaoパターンなら見たくない | jfluteの日記
気づいたら、ちょっとしたこともすごく時間がかかるようになって、なんだか働いてるわりには先に進んだ感が出ない...「なんか、この現場のコード直すお仕事、いやだなぁ...」メンテナンス意欲が下がり、別の仕事したくなってきます。
一方で、それに明確に気づかないのも問題です。ある日突然足が重くなるわけじゃなく、徐々に徐々にそうなっていくので慣れてしまうのです。一年前と正確に比べるのは難しいことです。効率悪いのに効率悪いと思わないでずっと...
人もどんどん入れ替わっていくので、初めて来た人はなおさらわからない。それに気づくのは優秀なエンジニアだけ。でも解決は困難。チームの問題意識がバラバラだから。
長期的デメリットのストーリー
最悪の長期戦へ。
「これ作り直そうよ！」しまいには、システムの再構築という、大きな代償を会社が払うことになります。でも、そんなコストを払えるか？そのためにビジネスを止められるか？
なかなかできません。そこで続くのは非効率メンテ、しかも長期化。優秀なエンジニアは敏感なので、どんどん立ち去っていきます。様々な理由を見つけて現場からいなくなります。
すると逆に、「やっぱり、再構築やろう！」と英断したところで、現場にスキルが足りず、再構築プロジェクトを進めることもできません。
両方だいじなんです
一方で、例えば "レビューが長くなる" からと言ってレビューせずにリリースをすると、早くリリースできた！と思って短期的に喜んでいたら、実は中期的デメリットを生み出すことになります。
しかしながら、コードの乱れでレビューが長くなることによって、レビューワーのレビューに対する心理的重荷が発生します。なので、自然とレビューに対して非消極的な対策をする空気になっていきます。これは一つの罠と言えるでしょう。
安易に逆サイドに振れる対策を取ると、行ったり来たりを繰り返すことになります。
なぜなら、"両方だいじ" なのですから。
=> 両方だいじなんです | jfluteの日記
バランスを見つける旅だし、両立を見つける旅なのです。
レビューがしやすければ？
逆に、レビューがしやすいコードであれば...
レビューが早い
(レビュー抜けが少ない)
(レビュー文化の維持)
↓
レビューでバグが発見しやすい
(システム品質の向上)
(緊急リリース削減)
↓
経験浅い人のコードもリリースしやすい
(マネージャー: 人員活用しやすい)
(ディベロッパー: 成長チャンス多くなる)
経験豊富な人も責任の重いコードを書きやすい
(仕組み周り、お金周りのコードなど)
↓↓↓ (月日が流れて...)
読みやすいコードの積み重ね
↓
既存コード分析時間の短縮
↓
メンテナンスモチベーションのキープ
↓
ビジネス意識のキープ
↓
ビジネス貢献への積極的な行動
つまり、先ほどのコードの乱れのデメリットを抑える効果があり、そして...
レビューがしやすいコードは、ビジネス意識の高いコード
と言えるでしょう。
"プログラミング環境の向上" の一つであり、非効率メンテの長期化によるエンジニアの離散を防ぎ、エンジニアのビジネス意識に下支えする重要な基盤となり、結果的に" ビジネス貢献の向上" につながります。
...
また、まだ経験の浅いディベロッパーであっても、とにかくレビューしやすいコードさえ心がけていれば、何かへんてこりんでも優秀な先輩たちが指摘してくれます。バグを見つけてくれます。それで問題は未然に防げます。それでビジネスを回していくことができるのです。
また、フィードバックをたくさんもらえることで、スキルアップのチャンスにもなり、指摘も少なくなってきて、早いリリースができるようになっていきます。レビューさえしやすければ、いきなり正解を引かなくてもいいわけです。
だから、まずはそこ。
...
...
...
ずばり言ってしまいましょう。
スキルが高くてもレビューしづらいコードを書く人には、プログラミング仕事は頼みづらいものです。
レビューしやすいコードとは？
大きくふたつ:
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」ってマニュアルが欲しいと言ってきたらどうしますか？
...
レビューワーへの気づかい
そこから思考の始まりです。
