ユアマイスター株式会社エンジニアブログ

ユアマイスター株式会社のエンジニアが日々徒然。

エンジニアがコードを見直すときに見て欲しい観点は「具体/抽象」「類推」「細部」の3つだ!

f:id:yourmystar_engineer:20180904144744j:plain

どうも。ユアマイスター星(@inase17000)です。

ユアマイスターでは、インターンエンジニアがたくさんいるのでコードのクオリティーを保つためにいろんな工夫をしております。 例えば、Sider(旧名:Side CI)を導入していて、静的解析をした上で自動でPull Requestに指摘コメントが入ったり、ルールを最適化していくための道具として使っています。詳しくは、下記のインタビュー記事をご覧ください。

sider.review

まだまだ人数もコンパクトなため、コミュニケーション自体は密に行えているのですが、ピザ2枚ルールだとギリギリの人数になって来たのかなという想いもあります。対象とするシステムは人数以上に広がったため、少しずつメンテナンスコストもかかって来ている印象で、定期的なリファクタリングとバグの修正を行なっています。

そもそもそんなことをさせなくていいようなコードのクオリティーを保つためにはどうしたらいいか、を考えてみました。

僕が先輩のエンジニアに教わったたくさんのことの中で、特に役立った3つを紹介したいと思います。

具体<>抽象のハシゴを上り下りする

ロジカルに会話したり、ロジカルにアルゴリズムを考えるときに必須な能力として、「物事を抽象的に捉えること」が挙げられます。

例えば、自分が担当している改修範囲のことばかり考えていて、そこの部分のテストはちゃんとしたのに、実は悪影響を及ぼす箇所があってバグを起こしてしまった、という経験は誰でもあると思います。(よく「デグレ」と呼んでいるものです。)

これは、自分が実装したい要件に対して、まさにそのものしか考慮に入れておらず、視野が狭い仕事をしてしまったことが問題だと言えます。

具体的な事象を、一度抽象化して問題の本質を捉えることを「ハシゴを上る」と例えています。 そして、ハシゴは「上る」だけではなくて、「下りる」ことも大事だったりします。 一度、抽象化した問題の本質を落ち着いて再度因数分解した上で解決のためのアクションを取ること。それを「具体<>抽象のハシゴを上り下りする」と呼んでいます。

この話をするときに思い浮かべるのが、「虫の目 鳥の目」という合言葉。(反対の意味のことわざだと、「木を見て森を見ず」というもの)

f:id:yourmystar_engineer:20180904140602j:plain

何かトラブルに巻き込まれたり、焦ってたりして、視野が狭くなってると思ったときには、自分が「虫の目」になっていると自覚した上で、空から見たらどうなってるかなを想像する「鳥の目」で上手く解決できることが多々あります。

多くの場合、この「鳥の目」を使うときに有効な方法は、以下です。

  • 5W1Hを考え直す
    • 例)このシステムが解決したいのはどんな課題だろう
    • 例)今回いくつも選択肢があった中で、これを作ってるのはなぜだろう
    • 例)なぜそのメソッドが必要なんだろう
    • 例)誰がその機能を使うんだろう
  • 他人に説明する

視野が狭くなっている時の傾向として、リリースまでの時間的なプレッシャーや、サービスの明暗を握る重要度的なプレッシャーと戦っていることが往々にしてあるので、まずはその上流工程を思い描くことで状況を俯瞰し、さらに他人と会話やチャットをすることで自分の言葉で説明することによって事象をさらに整理して伝えることができます。

たいていの課題は、因数分解ができたところで解決できているなんて話があるほど、複数の課題が絡み合った状態での戦いに悩んでることがほとんどなので、まずは「鳥の目」で状況を俯瞰し、今の行動が最適なアクションなのかを見直す、というのが必要です。

類推する

普段の開発の中で、結構多発するのが

あれ、ここはこう直してるけど、こっちは直さなくていいの?

という会話です。

大きく分けて2パターンあるかと思います。

  1. 同様の変数や処理を利用しているコードを全部改修できておらず、改修の範囲が不十分
  2. if文、else文、など条件分岐を使って一箇所にしか目を向けておらず、他方の処理の場合要件を満たさない

という形です。(他にもあるかもしれません。ぜひTwitterにReplyもらえると嬉しいです!@inase17000

1に関しては、もうずーっと言い続けてるのは「grepしろ」です。

変数名やメソッド名で、コード全体を検索して影響範囲を洗い出してから対応する。すごく当たり前のことなんですが、どうしても視野が狭まると、先入観で思い込んでる改修箇所に意識がいってしまい、本来直すべきところを見落としてしまう。

洗い出す→分類する→優先順位をつける→Just Do it という仕事の進め方が癖になっていれば、影響範囲調査をおろそかにすることはないと思うので、ここをひたすら言い続けることを大事だと思います。

2に関しては、少しエンジニアとして経験やセンスが必要になってくるかもしれません。画面上で見えてるコードに分岐の別パターンが見えてれば、よほど注意力散漫でない限り気づくとは思いますが、メソッド切り出しをされていたり、メソッドがオーバーライドされていたりしてコードの見通しが悪かったりすると、時にレベルがグッとあがります。

類推ができるようになるには、物事の対称性に気づく癖をつけるのが大事です。

例えば、こんなホテルみたいな廊下があったとしましょう。(絵が下手ですみません。)

f:id:yourmystar_engineer:20180904144513j:plain

手前左側から、101号室、102号室という部屋が並んでるホテルです。それぞれの部屋の中身を答えよ、と聞かれてももちろん超能力がない限り答えられないと思います。

しかし、101号室の中身を見せてもらえたとしたらどうでしょう。部屋同士のドアの間隔距離が同じだったりしたら、以下のような想像ができませんか?

f:id:yourmystar_engineer:20180904144525j:plain

つまり、同じ形の部屋が廊下の端までずっと続いているという構造です。そうであれば、きっと101号室のベッドと同じ場所に、102号室のベッドもあると予測できると思います。確証はないものの、何もヒントがない場合よりもずっと確度が高い予測だと思います。

ちょっと無理があるけど、101号室、102号室、任意でもう一部屋(例えば110号室あたり)を見ることができれば、さらに確度は高められると思います。(帰納法的アプローチ)

これをコーディングに活かそうとすると、こんな考え方を持ってみたらいいと思います。

  • インデントが同じコードは並列に記載されている可能性が高い。
  • 同じ見た目のHTML要素が他にある場合(同じclass名がスタイルに指定されている)、同じデータ構造を必要としている可能性が高い。
  • 1、2、3・・・と連続で処理が必要で、for文で書かれておらずベタガキの場合、最後の処理まで使われている可能性が高い。

例は色々あると思いますが、エッセンスとしては、似たような並列の要素に気づけるかが超重要で、それに対していくつかピックアップで検証して見ることで、類推しながら改修漏れを防ぐことができると思います。

神は細部に宿る

以前楽天で働いていたときには、開発部隊とは別にQAエンジニアのチームがありました。僕は開発部隊に所属していたのですが、一通り検証が終わったものを、QAチームに渡して、セキュリティテスト・機能テスト・シナリオテスト・複数デバイステストなどを織り交ぜて、効率的にリリース前にクリティカルなバグを見つけてくれていました。

テスト完了後に、最終的なQAのレポートも提出してくれるのでプロジェクトの規模に対してバグ発生率が高すぎないか/低すぎないか、などを議論した上で次のプロジェクトに繋げられる、すごくいい仕組みだったと思います。

ユアマイスターではQAチームはおらず、各自エンジニアが実装と検証を担っています。だいたいが既存仕様を知らず考慮不足だったり、影響範囲の把握ミスなどからバグが発生するので、そこは経験年数の長いエンジニアのレビューによって補うという体制です。

残り時間が少なってくると、どんなに真面目な人でも焦ります。 焦るとどうしても、妥協するポイントが生まれます。

例えば、

ここのclass名もう考えるのめんどくさいから適当な名前でいいか あっちでこう書かれてたから、ここもそれコピペでいいや

そんな誰にも気づかれない小さな妥協の積み重ねが、後々のメンテナンスコスト増加やバグ発生率上昇の危険に近づいていきます。

こればっかりは、複数人でチーム開発をしたことがないと得られない経験だと思うので、未経験者はへーそうなんだ、くらいの感覚でないがしろにしがちですが、経験者からするとどんな小さなことでもそれが氷山の一角で絶対に借金としては莫大になるということを把握しています。

これを回避するために必要なのは、先述の「具体/抽象」「類推」だけではなく、さらにもう一段階細かいところに目を向ける力と覚悟が必要です。 自分で運用していくサービスの当事者意識はもちろんですが、結局小さな小さな妥協が最終的にはお客さまに迷惑をかけることだったりサービスのイメージに傷をつける可能性があるという緊張感の下、行動に起こせるかが大事かと思います。

ある意味、コーディング時のコンプライアンスにかかっている。

各人の意識だけに頼るのも組織として成長していけませんので、ユアマイスターでは「セルフレビューチェックシート」を作り、各自がプルリクエストをレビュワーに提出する前に、自分でレビューする体制を敷いています。(セルフレビューチェックシートに関してはまたいつか書こうと思います。)

以上、なんとなくイメージが伝わりましたでしょうか。 なんだか書いてるうちに、自分に対しての戒めにもなっているなという気持ちになってきました。(笑)

そのうち、チームとして成熟していく過程で、どうしても発生するコードのクオリティーの問題、他社はどうやって解決しているのかを調査してブログにまとめてみようと思います!