去年買った本だけれどやっと手をつけて、読み終えた。
書いてある内容は、処理は細かくしようなど、基本的な事が多かったけど色々学びがあった。 特に印象が残ったこととして、
以上の3つがあった。
自分が入社した時によく、「言葉で説明できないものはつくれないから、まず何をするか考えて書き出してみよう」って言われていた。それと同じことなのですが、最近あまり意識してなかったのでいい振り返りになった。新しく入ってきた人に、お仕事をお願いするときに、何をするか書いてもらう => フィードバック => 実装 のような流れにしたら、実装の方向性とか迷わないで済むしいいなと思った。
いま関わってるコードでも、未使用のコードが少なからずあって、消すことで思わぬバグになったら嫌だなと放置していた。だけど未使用のものを残していても、管理コストが無駄にかかることになるし、初めてコード見る人が、どこでつかっているのかなと考えたりするので、本当に使うこと必要がないものは早々に消すべきだなとおもった。はやめに負債をなくそう。
テストはメソッド毎に、数パターン用意して書く、テストしやすいメソッドで書くとかあって、そのへんはだいぶ意識はしているけど、失敗した時のメッセージをわかりやすくするということが全く出来ていなかったなと気付かされた。実際に書いてしまっている良くない例だと
is $amount, $hoge * $fuga * 2;
のようなコードがあって、計算結果がamountと同じかどうかをテストしているのだけれど、このテストがfailしたとき、この1文がそのまま出力されるので、よくないなと。直すとしたら例えば
is $amount, $hoge * $fuga * 2, "{テストしたい内容}";
いたいにしたら、failした時わかりやすくなっていいなと思った。または計算の部分に名前をつけるとか。
読みやすいコードの基本的なところで、気づいていないところがいくつかって、今関わってるプロジェクトに浸透させたいけど、まずは自分が意識して書くようにして、綺麗だなと思ってもらうことから始めるのが良さそう。 今の自分には、人に綺麗だなって思ってもらえるようなコードかく力はないと思っているのでガンバクマツ。
この間他の人が作ったコードを実行したのけれど、そこにバグがあってつらーってなりました。 でもそのコード自分もレビューしてたし、なんで見つけられなかったのかなーって考えたことをまとめます。
まず、実装したものが本番にデプロイされて動くまでにいくつかのチェックが通ります。それぞれのチェックについて紹介していきます。
ここでは実装者がテストコードの作成と、実機を用いた動作確認をします。 大体のバグはここで見つかります。 またテストコードが作成された時点でプログラマ同士のコードレビューが入ります。 読みにくいコードはないか、実装が仕様とあっているか、負荷を考慮したコードになっているかetcをみていきます。
次に担当開発者全員で集まってチェックします。ここではバグはもちらん、作ったモノのクオリティをチェックします。 この時に、複数人で動かした際に生じるバグなどが見つかることがあります。 ここではクオリティ:バグ = 6:4ぐらいの比率で、チェックしていきます。
ここでは最終チェックです。担当開発者以外の第三者の目線からみてどうなのか確認するためにチーム全員でチェックします。ここではバグはほぼなくなっていてクオリティチェック、バグチェックの視点で見た時に 9:1ぐらいで、ほぼクオリティのチェックです。
ここでは専門の人テスターさんに、成果物のチェックをしてもらいます。ここでは仕様にそった実装になっているかをテストケースを元にテストしていってもらいます。 ここでは、あまり気づかないようなバグが見つかることが有ります。むしろそのためのチェックでもあります。
この時点で追加実装は完璧に動くものとして、既存機能が正常に動くかチェックしています。今回のバグは新規実装箇所でみつかるものだったのでここで見つかることはないですね。
見つからなかった原因を考えてみると、幾つものチェックが有るにせよ、今回のバグはコードレビューがしっかりできていなかったのが問題だったと思います。 コードレビューの質が落ちた原因として、
この二点が大きいです。どうしてもスケジュール的に余裕がない状態だと、コードレビューのに避ける時間は削られがち今回がまさにそのパターンだったかなと。 もちろんスケジュールによって、時間が避けないことはよくあることだとおもいます。テスターさんが稼働できる時間や、申請日など様々な要因で起こりえます。そのような場合全て実装し終わって、本番化するまでの間に時間などがあるなら、そこで追加の実装や、新しい施策に取り組み始める前に、たまっていた負債を返済するための時間を設けて、そこで本当に本番化していいコードなのか精査するすればいいのかなと思いました。※1 なんにせよ、急いでつくっても負債は返そうってことです。 次回から意識していきたいですねね。
※1: あくまでサーバーサイドのアプリの話で、申請が必要なクライント側のアプリだと同じようないかないですね...つらい。
最近single_or_createというメソッドをつかっていて、つらくなったのでメモ。
まずsingle_or_createとがどういうコードかというと、
sub single_or_create { my $self = shift; my $row = eval { #…insertの処理を走らせる }; if (my $e = $@) { # ..depulicatedしていたらsingleでinsertしようとしてたらデータを取ってくる $row = .. } return $row; }
こんな感じのコードになっています。 row無い時はinsertして、ある時はsingleで取ってくるのめっちゃ便利だなーとおもっていたのですが、これevalの中で行っているinsert処理は必ず毎回はしるので、アクセスが増えた時につらくなります。 なぜかdepulicatedな時は、insert走らないだろとおもっていたのですが、そんなことはなかったですね(╹◡╹)
なので書くとしたら
sub single_or_create { my $self = shift; my $row = … #…ここでrowを取ってくる return $row if $row; my $row = eval { #…insertの処理を走らせる }; if (my $e = $@) { # …depulicatedしていたらsingleでinsertしようとしてたらデータを取ってくる $row = .. } return $row; }
みたいな感じにして最初にrowを取ってくる処理を挟んで、本当に無い場合に、最初の処理を走らせればいいとおもいます。複数のユーザから1つのrowにアクセスいくようなときは、このような形で使うと便利かもしれません。
