コードインスペクターにあなたを愛させる方法

コード検査に関しては、人々は誰がそれを行っているかに焦点を合わせる傾向があります。ただし、コードを作成した開発者は、レビュー担当者と同じようにプロセスで重要な役割を果たします。検査用のコードを準備するための推奨事項はほとんどないため、作成者は単に無知から間違いを犯すことがよくあります。



この記事では、作成者としてコード検査に参加するためのベストプラクティスをまとめています。あなたが読み終えると、あなたは彼らがあなたへの愛をもって燃えるような完璧な変更のセットを検査官に送り始めるでしょう。

しかし、私はコードインスペクターが私を愛して輝いてほしくない

そして、彼らはまだあります。謙虚に。人類の歴史の中で、彼が生涯愛されすぎたと彼の死の床に不平を言った人はまだいません。

なぜコード検査を改善するのですか？

効果的な検査技術を学ぶことで、検査官、チーム、そして最も重要なことに、あなたの生活が楽になります。

• 加速された知識移転プロセス。一連の変更を適切に準備すると、退屈なスタイルの欠陥ではなく、成長する必要のある領域に検査官の注意が向けられます。そして、建設的な批判を大切にしていることを明確にすると、レビュー担当者はフィードバックにさらに力を入れ始めます。
• 他の人のための例。あなたの実行における良いコード検査の実践はあなたの周りの人々のための基準を設定します。通常、優れた手法が採用されます。つまり、同僚の誰かがレビューのためにコードを送信すると、より簡単になります。
• 最小限の競合。コード検査はしばしばチームに緊張をもたらします。責任を持って慎重にビジネスに取り組むと、紛争が発生する頻度が低くなります。

ゴールデンルール：レビューアの時間を大切にする

これは当たり前のようですが、検査官を個人的な品質管理の専門家として扱う著者によく出くわします。そのような人々は、エラーの少なくとも一部をキャッチしたり、チェックに便利な一連の変更を加えたりするために指を打つことはありません。



あなたの同僚は、限られた注意を払って毎日仕事に来ます。彼らがその一部をあなたに捧げると、彼らはもはやこのリソースを彼ら自身の仕事に費やすことができなくなります。彼らの時間を最大限に活用することは、単純な正義の問題です。



参加者がお互いを信頼できる場合、コードレビューははるかに成功します。あなたが彼の発言を真剣に受け止めることを彼が知っているならば、あなたの検査官は努力を惜しまないでしょう。審査官を克服する必要のある障害として認識した場合、審査官からの価値の低い推奨事項を受け取ることになります。

技術

1.



最初に自分でコードを確認します同僚にコードを送信する前に、それを読んでください。エラーをキャッチすることに限定しないでください-このフラグメントを初めて見ることを想像してください。それについて何が混乱する可能性がありますか？



私の経験では、コードの記述と編集の間に休憩を取ることが役立つ場合があります。人々は一日の終わりにコードの新しい部分を送信することがよくありますが、怠慢によって何かを見逃す可能性が最も高いのはこの期間です。朝まで待って、一連の変更を見直してから、それを同僚に渡します。







どんな馬鹿がそれを書いたのですか？

Cron Jobsと月のサイクルの同期：ETL



パイプラインを自然と調和させるために、同期ロジックを追加しました。検証者の作業環境を可能な限りシミュレートします。彼が行ったのと同じ比較ユーティリティを使用します。愚かなバグは、通常のコードエディタよりもこれらのユーティリティでよく見つかります。



自分が完璧だと期待しないでください。必然的に、バグ修正後に削除し忘れた部分や、削除しようとしていた漂遊ファイルを含むコードを送信することになります。それは世界の終わりではありませんが、それでも追跡する価値があります。間違いのパターンに気づき、それを防ぐためにどのシステムを実装できるかを考えてください。同じエラーが頻繁に発生する場合、検査官はあなたが自分の時間を大切にしていないと結論付けます。



2.変更セットの明確な説明を記述します



前回のメンタリングの仕事では、経験豊富なプログラマーと時折ミーティングを行いました。最初の会議の前に、彼は私が編集したソフトウェア設計文書を入手するように私に頼みました。彼に書類を渡したとき、私はプロジェクトが何であるか、そしてそれが私のチームの目標にどのように関連しているかを説明しました。私のメンターは眉をひそめ、率直に言った。「あなたが今言ったことはすべて、文書の最初のページにあるべきです。」



そして彼は正しかった。私は自分のチームの開発者の期待を込めてドキュメントを作成しましたが、部外者に読まれる可能性があることを考慮していませんでした。聴衆は私の部門の境界に限定されていませんでした-パートナーチーム、メンター、昇進について決定を下した人々がまだいました ..。文書は、すべてが彼らに明確になるように書かれているべきでした。この会話の後、私は常に理解に必要な文脈で自分の作品を発表しようとしています。



変更セットの説明には、読者が必要とする可能性のある背景情報を含める必要があります。レビュー担当者を対象に説明を書いている場合でも、レビュー担当者のコンテキスト情報が思ったより少ない可能性があることに注意してください。さらに、将来、他の同僚もこのコードを使用する必要がある可能性があります。変更の履歴を表示するとき、彼らはあなたの意図を明確にする必要があります。



変更セットの目的と、その方法を選択した理由の概要を説明します。



あなたは偉大な記述を書くの芸術に深くダイブしたい場合は、「読み どのようにGitがメッセージをコミット書くためにクリス・ビームによって」と「 私のお気に入りのGitはコミットデビッド・トンプソン」。



3.単純なものを自動化



するフローティングカーリーブレースや自動テストの問題などの小さなことを検査官に頼っている場合、彼の時間を無駄にしていることになります。







すべてが構文に合っているかどうかを確認しますか？私はコンパイラに目を向けますが、彼の時間を無駄にするのは残念です。



自動テストは、チームの一員としての標準手順の一部と見なす必要があります。検査は、継続的な統合環境での一連の自動チェック全体に合格した場合にのみ開始され ます。



チームが悲劇的に妄想的であり、継続的な統合を拒否する場合は、社内で自動監査を設定します。コミット前のフック、リンター、フォーマッターを開発環境に実装して 、すべての規則に従い、意図した動作がコミットからコミットまで持続するようにします。



4.コード自体が質問に答えていることを確認します



この写真の何が問題になっていますか？







機能の目的がわかりません。

そして、これはfrombobulateの実装がない場合に呼び出したときにFrombobulatorが渡された場合です。



著者は私が関数を理解するのを手伝ってくれましたが、次の読者はどうですか？変更の履歴を掘り下げて、すべての議論を読み直しますか？著者が私の机に来てすべてを直接説明するときはさらに悪いことです。第一に、それは私が集中するのを妨げます、そして第二に、生きている魂はもはや健全な情報にアクセスすることができなくなります。



検査官がコード内のポイントについて混乱を表明した場合、あなたの仕事はその人の状況を明確にすることではありません。全員の状況を一気に明らかにする必要があります。







- こんにちは？

-6年前にbill.pyを書いたとき、なぜt = 6になったのですか？

-お電話いただきありがとうございます。これは6％の販売税によるものです。

- もちろん！

-実装の決定を正当化するための優れた方法。



質問に対する最良の答えは、それを削除するコードリファクタリングです。ロジックを変更する方法を明確にするために、何の名前を変更できますか？コメントは許容できる解決策ですが、自然にそれ自体を文書化するコードの背景に間違いなく負けます。



5.変更を部分的に入力します



境界を広げることは悪い習慣であり、コードを検査するときによく見られます。開発者はロジックのバグを修正することを約束しますが、途中でインターフェイスの欠陥に遭遇します。 「まあ、とにかくこの作品を扱っているので、同時に修正します」と彼は思います。その結果、混乱が始まります。レビューアは、どの変更がタスクAで機能し、どの変更がタスクBで機能するかを把握する必要があります。



最良の変更セット は1つのことを実行します。変更がより正確で単純であるほど、検査官はコンテキストを念頭に置くことが容易になります。関係のない変更を共有することで、複数のピアに並行してレビューを依頼することもできるため、コードがより早く返送されます。



6.



機能的変更と非機能的変更を分離するスケーリングの制約から別のルールに従います。機能的変更と非機能的変更を混在させないでください。



コード検査の経験がほとんどない開発者は、このルールに違反することがよくあります。それらはある種の内訳の変更を行い、コードエディタはファイル全体のフォーマットを即座に変更します。開発者は、何が起こったのかを把握していないか、これがさらに優れていると判断してコードを送信します。コードでは、1つの機能変更が機能しない変更を含む数百行に埋め込まれています。







機能的な変更はありますか？



そのような混乱は、検査官の前で唾を吐くようなものです。フォーマットのみの変更は簡単に確認できます。機能の変更も。フォーマット変更の海での1つの機能障害は、無駄になり、狂ってしまう可能性があります。



さらに、開発者はリファクタリングを主要な変更にタイムリーに組み合わせるのが好きです。私は同僚の側でコードのリファクタリングを強く承認しますが、彼らが同時に新しい動作を実装する場合は承認しません。







リファクタリングの最中に埋め込まれた動作の変更



コードの一部でリファクタリングと動作の変更の両方が必要な場合は、プロセスを2つまたは3つのセットに分割する必要があります。

1. 現在の動作のテストを追加します（ない場合）
2. テストで何も変更せずにコードをリファクタリングします

レビューアは、第2フェーズでテストをそのまま維持することにより、リファクタリングが動作に違反しないことを確認できます。したがって、第3段階では、ここで何がリファクタリングされているのか、何が動作を変更するのかを理解する必要はありません。結局のところ、事前に一方を他方から分離しているのです。



7.大きすぎるチェンジセットを



分割する膨張したチェンジセットは、広大な境界の醜いいとこのようなもの です。状況を想像してみてください。開発者は、機能Aを実装するには、最初にライブラリBとCのセマンティクスを修正する必要があるという結論に達しました。変更がほとんどない場合は、何があっても、これらの変更が広がり、出力は変更の膨大なリストになります。



変更ログの複雑さは、影響を受ける行の数とともに指数関数的に増大します。変更が400行以上に影響する場合は、検査を要求する前にそれらをチャンク化する方法を探します。



たぶん、すべてを一度に行う代わりに、最初に依存関係を操作し、次のセットで新しい機能を実装することが可能になるでしょうか？機能の半分を今実装し、残りの半分を次のフェーズで実装する場合、コードを正常に保つことは現実的ですか？



コードを分割して機能するわかりやすい部分を作成する方法を考えるのは面倒な場合があります。ただし、これによりフィードバックが向上し、レビュー担当者の複雑さが軽減されます。



8.批判を親切に受け入れる



テスト全体を台無しにする最も確実な方法は、敵意を持って批判することです。多くの開発者は自分の仕事に誇りを持っており、自分の仕事を自分自身の延長として認識しているため、抵抗するのは難しい場合があります。そして、検査官が無知なコメントをしたり、人に行ったり すると、問題はさらに複雑になります。



最終的に、作成者であるあなたは、 コメントへの応答方法を自由に決定できます。検査官の言葉を、あなたの性格の評価とは何の関係もないコードの客観的な議論として扱ってください。あなたが防御的になるならば、それは悪化するだけです。



私は助けと利益を得るための試みとしてコメントをとるようにしています。レビューアがコードにばかげた間違いを見つけた場合、私は本能的に言い訳を始めます。しかし、私は代わりに彼の観察を控えて賞賛します。







-1900年1月と2月は、機能しません。

-まさに、よく知られています！



奇妙なことに、検査官がコードに明らかでない欠陥を見つけたという事実は良い兆候です。これは、スキャン用のファイルの準備に長けていることを示しています。ずさんなフォーマット、悪い名前、その他の目立つものがない場合、レビュー担当者はロジックと設計に深く入り込み、より価値のある観察を行うことができます。



9.検査官が間違いを犯した場合は辛抱強く



時々、検査官が単に間違っていることが起こります。彼はあなたがバグを植えることができるのと同じように完全に通常のコードを読み間違える可能性があります。そのような場合、多くの開発者は激しく自分自身を守り始めます。彼らは誰かが自分の仕事を批判していることに腹を立てており、何にも基づいていません。



ただし、レビュー担当者が間違っていても、まだ考えることがたくさんあります。彼が何かを読み間違えた場合、他の人が将来同じ間違いをする可能性はどのくらいありますか？読者は、見たバグがここにないことを確認するために、コードを前後に調べなければならないのではないでしょうか。







-すべてのNewNameLen文字を収容するのに十分なメモリが名前にあるかどうかの検証が実行されなかったため、ここにはバッファオーバーフローがあります。

-それは私のコードにありますか？考えられない！コンストラクターはPurchaseHatsを呼び出し、CheckWeatherを呼び出します。これは、バッファー長が一致しない場合にエラーをスローします。最初に200,000行のコードを読んでから、どこかで私が間違っていた可能性をあえて認めます。 ここですべてが正常であることを一目で理解できる



ように、リファクタリングする方法、またはコメントを追加する方法を検討して ください。誤解が言語のあまり知られていない機能の使用に起因する場合は、完全な知識を必要としないメカニズムを使用してコードを書き直してください。 10.わかりやすい反応を与える







私はしばしばそのような状況に陥ります。私は人にコメントを送り、彼はそれらのいくつかを考慮してコードを更新しますが、同時に彼は黙っています。不確実な時期があります。彼は残りのコメントを見逃したか、まだ修正中です。仮に、すでに行われたことを確認し始めると、このコードがまだ機能していることが判明する可能性があり、私は時間を無駄にしています。私たちが待つと、行き詰まる可能性があります-私たちは座って、お互いから次のステップを待ちます。



チーム内で特定のアクションの順序を確立して、現時点で誰が「バトン」を持っているかが常に明確になるようにします。何が起こっているのか誰も理解していないという理由だけで、プロセスが遅くなることを許すべきではありません。これは簡単です。変更セットレベルでコメントを残すだけで、行動する権利がいつ別の参加者に譲渡されるかが明確になります。







ヘッダーを修正する>コメントに従ってコードを更新する>すべての準備ができています。



確認してください。アクションを実行する必要があるコメントには、実行したことを確認して応答する必要があります。一部のコード検査ツールは、処理されたコメントにフラグを立てる機能を提供します。そうでない場合は、Doneのような単純なテキストの箇条書きのセットを使用します。コメントに同意できない場合は、訂正を控えた理由を丁寧に説明してください。







ReviewableやGerrittなどの一部のツールは、処理されたコメントにタグを付けることを提案しています



。レビュー担当者の努力に対して反応のバランスを取ります。彼があなた自身のために何か新しいことを学ぶことができるようにポイントを詳述したならば、あなた自身を「完了」のマークに限定しないでください。あなたが彼の仕事を大切にしていることを示すために思慮深く答えてください。



11.不足している情報を巧みに求める



検査官のコメントが解釈の余地を残しすぎることがあります。彼らが「この機能がわからない」という精神であなたに何かを書くと、すぐに疑問が生じます。それは、人には正確に何がはっきりしていないのかということです。長すぎる？名前は残念ですか？より良い文書化が必要ですか？



長い間、ぼんやりとしたコメントを明確にし、うっかり対立しないようにする方法を考えていました。私の頭に浮かんだ最初のことは、「彼女には何が理解できないのか」と尋ねることでしたが、それは不機嫌そうに聞こえます。



私はかつてこの種の同僚に漠然とした返事を故意に送りました、そして彼は彼の答えで私を武装解除しました：

「それをより良くするために何を変えることができますか？」

批判への寛容さと敵意の欠如を表現しているので、私はこのフレーズが好きです。さて、漠然としたフィードバックを得ると、「それをより良くするために何を変えるべきか」という基本的な動機のいくつかのバリエーションで応答します。



もう1つの便利な手法は、レビュー担当者が何を意味するのかを推測し、それに応じてコードを修正することで主導権を握ることです。 「理解できない」コメントが表示された場合は、コードを詳しく調べてください。通常、明快さの点で改善できる何かが見つかります。編集により、最終結果について異なる考えを持っていたとしても、変更を加える準備ができていることが検査官に通知されます。



12.レビュー担当者を優先します



テニスでは、相手に仕えるときにボールがアウトオブバウンズになったのかどうかが完全にはっきりしない場合は、相手に有利に状況を解釈するのが通例です。コードを検査するときも、このルールに従う必要があります。



いくつかの設計上の決定は好みの問題です。レビューアがあなたの10行の関数を2つの5行の関数に分割する方がよいと考える場合、客観的な真実はあなたのものでも彼のものでもありません。どちらのオプションが良いかは、好みによって異なります。



検査官が提案を行い、彼の立場を支持する議論があなたにとってほぼ等しい場合は、彼の見解を受け入れてください。結局のところ、2人のうち、彼はコードの新鮮な見方を利用する人です。



13.コード転送時間を短縮します



数か月前、私が管理しているオープンソースプロジェクトにユーザーが小さな変更を加えました。私は数時間後にコメントで退会しました、しかし彼はすでに終わりとともに姿を消しました。数日後、私は戻って、まだ答えがないことを確認しました。



6週間後、謎の開発者が修正を加えてプロジェクトに再登場しました。彼の仕事には感謝していますが、検証の2つの段階の間の長い休憩は、私が検査に2倍の労力を費やしたという事実につながりました。一般的に議論されたことを思い出すために、コードだけでなく自分のコメントも読み直さなければなりませんでした。その人が数日で現れた場合、追加の作業は必要ありません。







短い（左）および長い（右）送信時間

X軸-時間でのコード検査の知的コスト。 y軸-インスペクターメモリ。青い陰影-コンテキストの復元。青い塗りつぶし-コードチェック;灰色の塗りつぶし-待機中。赤い陰影-追加費用



もちろん、6週間は例外的なケースですが、チームで長く不当な一時停止が頻繁に見られます。誰かがレビューのために一連の変更を送信し、フィードバックを受け取り、別のタスクに気を取られたために編集を1週間延期します。



後でコンテキストの回復に時間を費やす必要があるという事実に加えて、半完成のコードフラグメントは複雑さを生み出します。それらは、すでに排出されているものとまだ排出されていないものを追跡することを困難にします。部分的に処理されたチェンジリストが作成されるほど、マージの競合が発生する可能性が高くなり、誰もそれらを台無しにすることを好みません。



検査のためにコードを送信したら、最優先事項はフォロースルーです。あなたの過失が原因でプロセスが停止した場合、あなたは検査官から時間を奪い、チーム全体の生活を複雑にしていることになります。

最終的に

提出する次の変更ログを準備するときは、対処できる要素を検討し、それらの機会を利用してレビューを生産的にします。コードレビューに参加するときは、プロセスを定期的に遅くし、リソースを浪費するものがないか確認してください。



レビューアの時間を大切にするという黄金のルールを忘れないでください。彼にコードの本当に興味深い部分に焦点を当てる機会を与えると、彼のコメントは役に立ちます。あなたが彼にささいなことに時間を浪費させたり、混乱を解き明かしたりすると、あなたとあなたの両方にとって非常に悪いことになります。



最後に、あなたの答えについて慎重に考えてください。些細なことや軽率な言葉に対する誤解は、驚くべき速さで物事を狂わせる可能性があります。他人の作品を批判することになると情熱が高まりますので、審査官があなたが彼を攻撃していると思ったり、適切な敬意を払わずに彼を扱ったりする可能性のあるものは慎重に避けてください。



おめでとう！ここまで読んだことがあれば、すでにコード検査の専門家だと思っているかもしれません。あなたの検査官はおそらくすでに発砲しているので、あなたは彼らを人間的に扱うべきです。