それでも厳しくレビューをする。妥協して技術的な負債をしない。

それでも妥協しないコードレビュー 雑なコードの罪深さ
目次

「動けばいっか…。」雑なエンジニアは意外と多い。

負債を気にしないエンジニア達

「動くしいっか…。」

そうやって、雑なコードを納品するエンジニアは結構多い。

また、そのプルリクをみて、「動くしいっか…。」雑にレビューをする人もまた多い。

彼らの多くは、

「〇〇さんだから大丈夫だろう」

とか

「そんな細かくレビューするのはかわいそう」

などと考えている傾向があります。

しかし、これは非常に危険です。

技術的な負債を蓄積させていき、雑なコードが複雑に絡み合い、簡単に改善できなくなります。

またエンジニアの育成の観点からしてもマイナスです。

だから、コードに妥協したくない。技術的な負債の罪深さ

膨らむ負債。妥協したコードを真似する人が生まれる

あるプロジェクトに参画したとき、妥協まみれのコードをみて驚きました。

それも先輩方がコードを書いていたので、指摘しづらい環境でした。

このとき、「もっと先輩でも厳しくレビューをするべきだった」と後悔することになるとは知りませんでした。

ある日、負債をつくった開発メンバー2人は別案件にアサインされました。

その代わりに、新たに3名がアサインされました。

結果、自分だけが案件を把握している人間となり、指揮をとることになったのですが、そのとき負債の恐ろしさに気づきました。

新しく参画したメンバーは、負債のコードを参考にして実装し始めました。

結果、負債となるコードが大量に生まれました。

まさに負債だなと、このとき痛感しました。

わざわざ実力を下げてコードを書くエンジニア達

もちろん、コードレビューで雑なコードは全部弾いて、細かく指導をして改善させました。

そのとき、新規に参画したメンバーの理解度をヒヤリングをしました。

中には歴が長いエンジニアもいたため、違和感を感じたためです。

すると衝撃な事実が反映しました。

「やっぱりそうですよね!!いやー悪いコードだなって僕も思っていたんですよ」

このとき、技術的な負債の罪深さを痛感しました。

そう、案件のコード文化に合わせて実装する。これはエンジニアの一つの思想だと思います。

この思想が仇となり、負債となったコードを真似する人が多く誕生したのです。

つまり、彼らは自分達の実力をわざわざ下げて実装していたのです。

せっかく丁寧で良質なコードを書けるのに、雑なエンジニア達の基準に合わせて書いていたのです。

非常にもったいない。

このことをキッカケに何を言われても負債は産みださい決めました。

雑な価値観が癖づいたエンジニア

非常に残念だったのが、ポテンシャルが高いエンジニアだったのに、

長く雑な価値観・基準で開発をしていたため、それが浸透してしまったエンジニアもいました。

「そこまでしなくてもいいんじゃないの?」

この癖から抜け出せないようになっていたり、その問題点を疑問にも感じていない様子でした。

これは非常に罪深いです。

彼らの輝かしい可能性は、この価値観によって光を失いました。

そして目先のことだけ考える、先をみていないエンジニアを大量に生まれてしまいます。

コードの負債も、負債となる雑な価値観もなくすべきだと結論づきました。

負債を解消する負債

再度テストし直す負債

レビューの段階で改善しておけば、一度のテストで済んだのに、後から負債を改善するために、全体を修正してテストもそれぞれやり直す。

実装が進めば進むほど、この工数も大きくなります。

そうなると、QAエンジニアの負担も大きくなります。

少なくともエンジニア1人は負債解消に集中するので、開発メンバー1人を削った期間が生まれます。

その間は、他のメンバーに開発のタスクが集中します。

つまり、ただただ二度手間で、人員を割かなければならないので、人件費としての負債も大きいわけです。

実装中のタスクに影響する負債

また、改善させたコードを反映しづらいこともあります。

反映された雑なコードを元に次の実装を進めているため、負債を解消させたコードを入れると、
それに合わせてコードを書き直さなければいけません。

そうならないように設計されていればいいのですが、雑なコードを書く人はだいたい考慮をしていないので、当然影響を受けます。

じゃあ、いつ負債を解消させるのか?という話が生まれます。

「実装するタスクが落ち着いたら、負債を解消していこう」となります。

しかし、その頃にはたくさんの機能が実装されており、さらにリファクタリングしないといけない範囲が広がります。

結果的にテストも実装も工数が大きくなります。

まさに負債です。

このことから、始めから負債をつくらないのがベストだとわかります。

先輩相手でも詳細にレビューをする

これらの気づきから、先輩相手でも詳細にコードをみて、

  • 【可読性】レビュアーがコードを眺めているだけで、仕様が伝わるコードになっているのか?
  • 【技術面】この実装の場合、どういった不具合が生まれる可能性があるのか、仕様変更に耐えられない実装方法になっていないか?

これらを念入りに確認しながらレビューしています。

どれだけ信頼できるエンジニアでも人です。

睡眠不足や二日酔いなど、日によってパフォーマンスは異なります。

当然ミスはしますし、集中力が落ちて雑なままレビューに出すこともあるでしょう。

だから、「この人だから大丈夫だろう」などと人ベースで判断するのは危険です。

人ベースではなく、機械ベースで判断する

具体的な改善点として、

  • 静的解析ツールである「PHPStan」を導入。
  • テストコードをしっかり書く。

これらを実施して、コードの質を担保しています。

理由としては下記になります

  1. 人からレビューされるとショックを受ける人もいる。でも機械に指摘されると傷つかない。
  2. レビュアーのレビュー時間を削減できる。開発に時間を使える。
  3. 人だと見逃す書き漏れ、書き間違いを解消できる。
  4. DB構成などのレビューに時間を使いやすい。

これらの理由から静的解析を取り入れています。

最近だと静的解析がデフォルトでされる言語が主流になっているので、それらを採用した開発がベストだなと思います。

他にもE2Eテストを見直しするなど、改善方法はいくらでもありそうなので、模索していきたいと思います。

ぎゅう
WEBエンジニア
渋谷でWEBエンジニアとして働く。
LaravelとVue.jsをよく取り扱い、誰でも仕様が伝わるコードを書くことを得意とする。
先輩だろうがプルリクにコメントをして、リファクタしまくる仕様伝わるコード書くマン
よかったらシェアしてね!
  • URLをコピーしました!
  • URLをコピーしました!

コメント

コメントする

目次
閉じる