伝わらない命名をしない
伝わらない命名をしない
- 名前はとても大事なものです、ロジックに反した名前を付けてしまうとレビュアーは？となります
- 『プログラマが知るべき97のこと』の日本語訳版でmatzさんも寄稿されていましたね
どうやるか？
- Google翻訳を使わない
- 本質的な名前を導き出す
- それを表現するべき単語があればそれを採用する
- 中学生で習う英語を思い出しましょう、長いものでもSVOCやSVOOです
- 誰が(S)の部分はおそらく現代のプログラミングであればクラスとして省略可能です
- つまり、我々が表現すべきなのはVOCやVOOの部分であるべきです
主語を明確にする
- 先の項で述べたS(主語=誰)が主体です
- まず、これを明らかにしないことは責務に取り組むことが出来ていない証左です
- 誰が、何に対しての責務を持っているのかを設計段階で明らかにしておきましょう
それが何をするのかを明確にする
- 主語は決定しました
- 次はV(動詞)を決定しましょう
- あなたの関数は主語が行う何らかの動作を行うものです
- 関数名の先頭には動詞を持ってきましょう、それが全てです
- また、プロジェクトメンバーの同意が取れるのであれば、引数が目的語や補語となるとなおよいですね
冗長な名前を用いない
- 主語が明らかになれば、あとは振る舞いを決定するだけです
- 主語を表す単語がメソッド名に現れることは不要な複雑さを生み出します
- これらはあなたのコードを読む人間を混乱に陥れる罠ともなります(別の主語がそれを使うなど)
変数名はどうか？
- クラスとその振る舞いが決定すれば、端的な名前を付けてあげることは容易になったはずです
- 端的に、本質的な名前を付けられる様に日々努力しましょう
伝えたいこと
- S(誰が)をクラスとして扱える様にする
- V(何をするのか)をメソッド名の先頭にもってくる
- O(目的語)やC(補語)はそれらを補完するだけのものである
- これらの命名がしっかりとできれば変数名の命名はそれに従うだけとなる
- 主語(クラス)と同じ単語が出現する様なメソッド名は冗長である
- 私は英語学の研究者の卵だったこともあり、このあたりは出来ていないなと思うコードによく出会います
- その原因の多くは正しくその変数や関数の正体を見極めていない命名にあります
- 直訳は正しい認識を阻害します。正体を見極めすべき命名を正しく行いましょう
問題を解決するためだけにコードを書く
問題を解決するためだけにコードを書く
- 当ブログで一番伝えたかったことです
コードを書くのは最終手段
- コードは必然的に書かなければならない時にのみ書かれるべきです
- なぜならば、不要なコードは常に単なる複雑さを提供するだけのものだからです
- バグを生まない最良の手段はコードを書かないことです
そのコードが必要であるかを確認する
- 書かなくても良いコードを書いていませんか？
- もしくは、書いたはいいが置き換えされてしまうコードを書いてはいませんか？
- もしもあなたのコードがこれらを満たすのであれば、仕様や設計、ロードマップの見直しをしましょう
- 私自身前職で2週間ほど掛けたコードがリリースされないという憂き目に遭いました
- 原因としては仕様に対する無知と関係者への早期な設計レビューを依頼しなかったことが挙げられます
書かれるべきでないコードとは？
- 置き換えが予定されているコード
- 設計の段階でしっかりとヒアリングされずに書かれたコード
- そもそも設計が為されていないコード
- また、設計段階でレビューがなされなかったコード
- 正しく要件を満たすことが出来ないコード
- 要件を満たしてはいるが、別の関心事が発生した際にとりあえず書かれるコード
- テストコード等に関しては別の機会に書きたいと思いますので割愛
私の立ち位置
- エンジニアがすべきことは誰かの問題の解決を図ることです
- ここでいう問題とは主に以下の二つです
- 誰かがシステムの不備で困っていること
- 誰かが猥雑な仕事を余儀なくされて、本質的な問題に取り組めないこと
- 上記の二点を解決すること、そうあり続けるべく研鑽を積むことが良いエンジニアのあるべき姿だと信じます
よくない対象
- コードを書く上で向き合うべき対象は困っている人間だと信じます
- つまり、下記の様なものがコードを書く目的であれば見直しを図るべきです
- 上司からいわれたので従う、要件とか誰のためかとかは知らない
- リリーススケジュールが決まっているのでそれを満たす
- 上記の様なことが発端で生み出された場合は、概ねおぞましいコードを世に放つ要因となりえます
- これらの問題の共通点は本質的な問題に向き合っていないことです
- そして、多くの場合においてそれは共有不足から発生しうるものです
伝えたいこと
- エンジニアの本質は問題解決
- 誰かが困っていることに向き合おう
- 困っている誰かに向き合おう
- 仕様を正しく理解することで説明責任を果たせるのであればコードを書く必要はない
- あなたがコードモンキーでないなら何のためにコードを書くのかを正しく理解することから始めましょう
ライブラリのコードを隠蔽する
ライブラリのコードを隠蔽する
- 私は今、PHPでLaravelを使うプロジェクトに参加しています
- ジョイン当初に驚いたのは下記の様なコードが散見されていたことです
return response()->json($result);
論点は2つ
どうすればよかったか？
- ライブラリのコードは問題があったとしても不可侵であるべきです
- 問題が発生するリスクをヘッジするため、ライブラリのコードを利用するラッパー関数を準備しましょう
- プロジェクトでライブラリにアクセスする場合はそのラッパー関数を利用する様にしましょう
- これはフレームワークに限ったことでなく利用しているサードパーティのライブラリ全てに適用されるべきルールです
- ライブラリは先人の知識の結集であり、とても尊いものです
- しかし、どんなに優秀な人でもミスを犯し得ます
- また、宗旨替えをしてシグニチャや命名規則を変更することは容易に予測できます
伝えたいこと
条件分岐の順番にも気を使おう
条件分岐の順番にも気を使おう
- こういった条件分岐を見るとモヤモヤします
if ($foo->exists()) { // DBにレコードがなければFALSE return false; } elseif (isset($parameter['bar'])) { // パラメータにbarが含まれなかった場合はFALSEを返す return false; } else { return true; }
何がマズかったのか？
- DBにクエリを発行する処理と
issetで値検証するだけの処理では前者の方が高コストです - コストが低い処理から順に条件を判定していくべきです
- 前提条件としてコストが高い処理が含まれる場合に無理に行う必要は当然ありません
コストが高いものとはどういうものか？
伝えたいこと
- 不要なネットワーク接続やシステムコールを伴う処理はコストが高いです
- 一方でメモリにのったでーたの判定は非常に高速です
- 不要なコストを発生させないために低コストなものから判定していく様にしましょう
例外を拾おう
例外を拾おう
- 正確には正しく拾い、投げ捨てることをしない様に努めましょう
- こういうコードを見るととてもモヤモヤします。そして不安になります
try {
// 例外が発生するかも知れない処理
} catch (Exception e) {
}
何がよくないか？
- まず、例外が発生したにも関わらず対応策をとらないのかとても気になります
- たとえば、齟齬の発生しているDBから取得した値が原因になっているとして
- そのデータやそれを生み出すコードを特定する機会を逃すことに他なりません
- 例外を発生させることはシステムが不調をきたしていることを知らせてくれるものです
- 故障したスポーツ選手が痛み止めを使って試合に出続ける様なものです
- 問題が大きくなる前に正しく状態を知り、改善することが最終的に良い結果をもたらすでしょう
そもそも、例外とは何なのか？
- 例外とは処理を継続することが不可能になった状態です
- 例外が発生したら速やかに処理を中断すべきです
- そして、ユーザにエラーの発生を伝える、または管理者に通知を送信することを心がけるべきです
- まずはエラー内容をログに出力しましょう
- スタックトレースを読めばある程度の状態を知ることができます
- 発生箇所やどういった属性のユーザが何にアクセスしたのか知ることも助けとなるでしょう
- エラーの原因にたどり着くためにできるだけ多くの情報を残しましょう
多くのエラー
- 日々、膨大なエラーログが出力されるとすればシステムが不健全であることを示しています
- 少しずつでもエラーの原因を正しく見極め改善しましょう
- あまりにも発生頻度が多く軽微なものあれば、ひとまずは絆創膏を張ってふさぎましょう
- そして、他の致命的かも知れないエラーに隠れ家を提供しない状態を作ることが大切です
- 回収にかかるコストとリスクの度合いを考えて優先度を決め賢く投資しましょう
伝えたいこと
- 例外はシステムからの助けての合図です。その状態を解決しましょう
- 例外が発生したら早い段階でそれに気付く仕組みを導入しましよう
- チャットなどリアルタイムでそれがわかればベストですね
- そして、通知がこない状態を常に維持することが出来れば本質的な問題に取り組むことができます
- 溢れ続けるエラーログも日々改善を続けることで少しずつ小さなものになるでしょう
- 大きな問題へ発展する前に正しくエラーを改善し続けましょう
UNIXコマンドを味方にしよう
UNIXコマンドを味方にしよう
- 今日はレビュー自体の話でなく、レビューを依頼する際にしておきたい事前準備の話をしたいと思います
IDEやGUIツールを窓から投げ捨てよう
- IDEやGUIツールは非常に便利です。否定はしません
- ただ、こういったツールには一つの弱点があります
- それは、検索結果やなぜ不要なコードであると判断したかの基準が機械任せになることです
- また、異なるIDEを使っている場合はその特性まで理解する必要があるかも知れません
- 更に不幸なのは、その結果が見やすいリストとして出現してしまうことです
- それ自体は問題ありません、ただ、それをレビュアーにどう伝えますか？
UNIXコマンドは再実行可能
- たとえば、障害対応のチケットに下記の調査方法があればレビュアーの負担が減ります
- 問題が起きたのでxxxでgrepした
- git-blameしたところ、この変更点が問題の様だ
- git-showしてみたが他にもバグにつながりそうな箇所はyyyとzzzだ
- さらに調べたが、zzzには影響がありそうだがyyyは問題なさそうだ
- 上記を踏まえてyyyを実行するロジックはそのままにxxxとzzzを実行する処理を新たに作った
- レビュアーはこれらのコマンドがチケットに記載されていればそのコマンドをまずレビューすることから始めます
- そして、それらのコマンドが検証通りであることを確認して初めてレビューを始めることが可能となります
- これらの情報がなければあなたはなぜ、その行動をとったのか説明せざるをえないでしょう
IDEの検索を使っている場合に起こること
- 説明責任を果たせない、もしくはスクリーンショットをとり貼り続ける日々を余儀なくされる
- 数年前までSIerにいたので、こういった仕事をしている方は何人か知っています
- ただ、そういった仕事をされる方々はコードレビューを依頼する働き方をしている人間とは違う目的を持っています
- また、コードレビューを依頼する側の人間がIDEの検索結果を頑張って貼り続ける姿はあまり見たことがありません
- テキストのコピー&ペーストは簡単です
- 対して、画面のコピー&ペーストは今なお猥雑です
UNIXコマンドを活用しよう
- もし、IDEの検索機能で調査しているなら今すぐにでもgrepコマンドに置き変えましょう
- あなたが何を検索し、たとえば小文字の考慮が漏れているねといったことにはレビュアーが気付いて手戻りが減るかも知れません(たとえばPHPの場合は関数は大文字小文字を区別しません!!)
- また、どのコードを探索し、その結果としてバグの原因であったかを数ヶ月後に知ることを可能にするパンくずともなりえます
- grepコマンドを利用し、自分の足跡をしっかりとメモしましょう
- ファイルの場所がわからない？findコマンドを使いましょう
- 同名のファイルがあるなら、パイプに食わせてxargsでgrepすればお探しの関数はすぐに見つかるでしょう
- Gitを使っていて、変更点を探索したいのならばgit-logやgit-blameやgit-showが助けとなってくれます
UNIXコマンドを使った調査結果はテキストをもたらす
- もはや、あなたはチケットにそのコマンドの実行結果をコピペするだけで説明責任の一端を成し遂げました
- Macを使っているのであれば、パイプにpbcopyを食わせてやるだけでクリップボードに内容が記録されます
- あとはペーストするだけです
- 調査結果を記録することはあなたが間違った時、どういう調査をしてもらいたいかを示したい時に役に立つでしょう
- IDEやGUIツールはたしかにあなたの助けとなっているでしょう
- ただ、そういったツールたちも内部的にはこの様なコマンドを叩いています
- そういったツールが成し遂げる内相を理解して使いましょう
- でなければ、あなたはチールを使う側でなく、ツールに使われる側です
伝えいこと
- ツールが暗黙に行ってくれることに甘えず、記録者として振舞いましょう
- ツールで調査した結果を横で見せられる時間を喜ぶエンジニアはそれほど多くはないでしょう
- 自身の調査を正確なテキストに落とし込むことであなたの同僚の費やす時間が減ります
- また、あなたが説明に割く時間も短縮されるかも知れません
- 調査のログをとりましょう
- そのログはきっとあなたの将来のなんでこの時にこうおもったんだっけ？を解決してくれます
- そして、今から7年前に行動を元にこれを教えてくださったSさんにこの場を借りて感謝を。。
コードフォーマッタを使おう
コードフォーマッタを使おう
- インデントや、スタイルの指摘は機械に任せようという話
規約を破るのが人間
- コーディング規約に沿わないコードを書くこともある
- コーディング規約にないからと時代遅れも甚だしいコードを書くこともある
- うっかり、スペースを消してしまうこともある
- POSIXに乗っ取らず、行末改行を入れないこともある
これらは機械に任せよう
- インデントの指摘ばかりをされても当然ながら不愉快でしょう
- わかる。すごくわかるが後で入ってくる人間の精神衛生を考慮すれば多少の我慢をしてもらいたいです
- 行末の改行などはエディタの設定を見ればだいたい入っています
- インデントのチェックツールは世の中には腐るほどあります
- 自分が使っているのは(PHPなので)、php-cs-fixer
- こういったツールを使うことで若干の違和感程度まではコードの違和感を取り除くことが可能です
- また、カスタマイズすることで少しずつ自分たち好みのフォーマットに落とし込んでいくことが可能でしょう
見た目の問題はレビューの本質ではない
- レビューの打ち返しがレイアウトに関することだけであれば、そのレビューは不毛なものとなるでしょう
- 単にレイアウトの確認を行ってもらうためにレビュー依頼したはずではないでしょう
- レイアウトに関する指摘をされない様にコードフォーマッタを使いましょう
- 不毛な議論がなくなるだけでレビューが少しだけ健全なものになります
どうすればうまく回せるか？
- 自分はつい最近、php-cs-fixerの適用を行い、phpmdを使って静的解析させるシェルを書きました
- 更にこのシェルが最終的にレビュー依頼のテンプレートを吐き出すことにしました
- レビューの前にレイアウトの修正や自身のコードの不完全な部分を見つけられるのでなかなかに気に入っています
- 何かのついでにコードフォーマッタや静的解析ツールを使える状態に持ち込めるのは案外いいものですよ
