コード品質についての社内勉強会第二回

1.前置き
2.それぞれの発表について

● TS「通処理修正による意図しないエンバグを防ぐ方法」
● ST「Assersion」
● yano「三項演算子、ガード節、引数のBoolean」
● TK「型は証明」

3.まとめ

Contents

前置き
それぞれの発表について
まとめ

前置き

こんにちは。
エンジニアをしているyocです。
前回に続き、コード品質勉強会の第二回が開かれました。
僕にとってはシニアエンジニアたちの経験値を吸収できるいい機会の一つでした。
せっかくなので自身へのさらなる定着を目指しつつ、僕の視点でまとめつつ振り返っていきたいと思います。

それぞれの発表について

共通処理修正による意図しないエンバグを防ぐ方法

一人目の発表者はTSさん、お題は「共通処理修正による意図しないエンバグを防ぐ方法」でした。
まずずばりその方法とは、

です。
まず1について、
例にあった消費税と自動車の排気量超過時税率を現在が同じ10％だからといって同じ定数として共有すべきでないというのがしっくり来ました。
概念が異なるものだし、いつか片側だけ値が変わってしまうということもありえるからですね。
また、おそらく一番危険なのは後から共有しようとする時で、
コードの中にZEIという定数があってその値が10%だったらついうっかり消費税だと思って引用しちゃったりしそうな気がします。
返り値や値だけ見て概念のことを頭ごなしにすると痛い目を見る可能性があるということです。
抽象度が高い名前がついている場合は特に注意が必要ですね。
なのでまず2つの多少似た処理があった時、何も考えずに一つにまとめてしまうのではなく、
本当に一つの関数としていいのか、その違いを引数で表現できるか、別々の関数を用意するか、そもそも新しいクラスを作るべきか、
そして、命名でしっかり概念を伝えられるか、微妙そうならコメントは十分か、
などを考えようということです。
次に2です。
まず設計時にそもそも1を気をつけていなかったとか、予想が効かない概念の追加変更などがあってしまった場合、
共有化の具合や利用先がどのような思いで利用しているかを把握できていないと
修正するとどうなってしまうか確信しきれないこともあります。
その時は改めて新たな関数を作るということも大切だということです。
そして3です。
結局チーム開発では意識をチームで共有していないと効果は薄いです。
例えば1で無駄に切り分けしてしまうと、自分は満足しても他の人にとっては無駄が多くて煩雑で触りづらく把握しきれないといったことにも繋がります。
共通処理に限らず、他人の視点というのは重要ですね。

Assersion

二人目はSTさん。お題は「Assersion」です。
簡潔には事前チェックと事後チェックをして、AssersionErrorというちょっと特別な例外を投げることです。
事前チェックとは関数の引数に対して行い、後の処理が正しく行われるための前提条件を書いて、そぐわない場合は例外を投げるようにすること、
事後チェックとは返り値を返す前に要件を満たした返り値を生成できているかということをチェックし、そぐわなければ例外を投げるようにすることです。
これらを書いておくことで、ミスの特定や被害の減少につながります。
また、そもそもどういうものを扱っているのか、特に重要な条件はなにかと言うのがコードからも読み取れるようになっていいのではないかと思います。
Assertionは言語によって特に流儀が違う所で、言語だけでなくモジュールで提供されているものもあります。
本番環境でAssesionErrorによって全てが落ちてしまうのが困るなどといった場合は、
エラーを出しつつ止めることなく動かすといったことも切り替えできるようです。
関数単位で処理の整合性を担保できる手法の一つなので、
しっかり使いこなせるように、面倒臭がらずに書いていきたいところですね。

三項演算子、ガード節、引数のBoolean

三人目はyanoさん。お題は「三項演算子、ガード節、引数のBoolean」です。
最初は三項演算子。
一般には「condition ? a : b」というやつですね。
一応正確には条件演算子ですが、他に三項を取る演算子があまりにもないので、三項演算子といえば?:を指すことが多いです。
conditionの真偽によってaかbかの値を返す式ですが、
3つ以上の分岐を三項演算子だけで書こうとすると非常にわかりづらいという欠点があります。

alpha == bravo ? charlie : delta != echo ? foxtrot : golf

でどの条件の時に何が返ってくるかはぱっとわかる人は少ないと思います。
一応、インデントを揃えて

alpha == bravo  ? charlie
: delta != echo ? foxtrot
: golf

のようにすると読みやすくなるよというTSさんのコメントがありました。
正確にはTSさんは:の直後に改行と言っていましたが、そうすると最後の値が浮いてしまうので上記の様な感じがいいかなと思います。
僕なら2つは三項演算子でさくっと、3つ以上はswitch構文などに譲るようにしますね。
ただ、三項演算子に限らずあまりにも横に長い式は良くないし、適宜インデンテーションを行おうというのは賛成ですね。
また、今回は話題にのぼらなかったのですが、
気になったので三項演算子の演算順位について調べてみました。
基本的に非常に優先順位が遅い演算子のようで、

x * flag == 0 ? 3 : 5 //(x * flag)が0なら3、そうでないなら5
x * (flag == 0 ? 3 : 5) //flagが0ならxを3倍、そうでないなら5倍

のようになります。ただ、言語によって異なるかもしれません。
三項演算子自体は便利なことも多いので使いますが、
式が長くなってくると途端によくわからなくなってくるので、そういうときは避けたいと思います。
次はガード節。
まずはガード節でない書き方。

if (number <= 0) {
  return 0;
} else {
  ... //numberが正のときのみ実行される処理
}

次はガード節を利用した書き方

return 0 if number <= 0
... //numberが正のときのみ実行される処理

初めてガード節について聞いた時は一番最初に例外条件を除けば良いとだけ聞きかじっていたので、
if…elseのif側を異常系に、else側を正常系にするといいみたいに考えていました。
ちょうどガード節でない書き方をこれもガード節だと思っていたんですよね。
だってガード節という名前自体が最初に異常系を取り除こうという感じがするじゃないですか?
なのでガード節のいいところはelse節をそもそも使わないことによってネストを減らすことができるということに驚きました。
実際エディタを横に分割しているとネストが深くなるとすぐに左右が厳しくなって見切れたりするので、いい考え方だと思います。
今まではなんとなくifに対してそうでない場合は必ずelseを書かないと文体として気持ち悪いと思っていましたが、
ネストを減らすことができるという利点には勝ることはないと思うようになったので、改めるようにしたいです。
最後に引数のBooleanです。
例えば定義元においてはdef set_state(is_online)のようにネットワーク接続状態を保存している関数なのだなとわかりますが、
実際の使用場所をみるとset_state(true)のようになって何もわからなくなってしまう。
そこでその解決としてStateType::ONLINEのようなenumを用意してset_state(StateType::ONLINE)とすると呼び出し元を辿らずともなにがしたいかわかりやすいよねというものです。
他にもキーワード引数といってset_state(is_online: true)のようにするとか、そもそも関数名をわかりやすいものに変えるとかといったこともあります。
もちろんBooleanに限らず一般、ゴールド、プラチナユーザのように3つの状態を扱う場合にintで0,1,2を使ってしまわないなど色々応用の聞く話でもあります。

型は証明

最後の発表者はTKさん。お題は「型は証明」です。
DDDにおけるvalue objectのように、型そのものにvalidationを追加してどういう物を扱っているのかを規定するというものです。
例えば郵便番号をただStringで扱うのではなくMyPostalCodeクラスをつくって、正規表現/^\d{7}$/にマッチするときのみ扱えるようにするなどです。
これのいいところはStringだと実際に値を見ないと”123-4567″か”1234567″か”１２３ー４５６７”か、
どの表現で入っているかが全然わからないところが”1234567″が入っていると規定されることです。
そもそも入力時のバリデーションをフロントエンドからバックエンドまでのどこでしているのかをしているのも苦労ですからね。
また型自体に”1234567″を”１２３−４５６７”に変換して出力する機能を持たせておくことで、
療法が必要な場合でも変数として”1234567″か”１２３ー４５６７”が混在するといったことも避けられます。

まとめ

最後にこれらの話のまとめをしたいと思います。
いずれに関しても思うことは書いた当時と後で見返す時では全然違うということですね。
書いてる時というのは例えば三項演算子使って1行で書けたぜカッケーとか、
わざわざクラス作らなくてもintで十分じゃんこんなのなどと思ってしまったりするわけですが、
残念ながら見返すと大抵多少長くてもわかりやすく書いてくれとか、
修正時にエラーが出ているけど原因箇所がどこだかわからんと思ってしまうわけです。
だけどその一方で、文句をいいながらもわざわざ書き直すほうが手間だな、動いてるじゃん、なんとか修正できちゃったな、という自分もいます。
やっぱり失敗しても一度きっちりやってみないとその良さがわからずそのまんまということが多いので、
その点でこの勉強会は経験値を簡単に稼がせてくれるので僕にはとてもありがたいです。
次回もまた期待です。