[Ruby/Rails] 例外で深くなったネストをGuard Clauseですっきりさせる

こんにちは、hachi8833です。 Guard Clauseとは Guard Clauseは、条件分岐のネストを深くしないための技法のひとつで、「ガード節」「ガード条件」「ガード構文」などと訳されることがあります。その動作から早期復帰と呼ばれることもあります。 Guard Clauseの考え方を言葉で表せば、「引数が主に例外的な条件で例外的な値を返すのであれば、処理の冒頭部分で条件判断とreturnを行って例外処理を完結させ、メインの処理に影響を与えないようにする」ということになります。Rubyに限らず、多くのプログラミング言語のリファクタリングで有用な実装手法です。 始めはシンプルな例外処理であっても、改修が進むに連れて例外処理が深くなっていくことがよくあります。処理の分岐先の処理がどちらも同じぐらいの重みがあれば、通常どおり分岐するのが自然ですが、特に例外を事前にチェックするような処理であればGuard Clauseを積極的に使うことで条件分岐のネストを浅くでき、コードが読みやすくなります。事前処理する例外の数が多いほど効果を実感できるでしょう。 Rubyでは後置のifや後置のunlessが使えるので、以下のようにGuard Clauseを1行で簡潔に書くことができます。なお、後置のifはPythonやPerlなどでも利用できます。 #<略> return if user.blank? #例外処理をここで完結させる #メイン処理をここに書く #<略> Guard Clauseによるリファクタリングの例 リファクタリング前 def invalid_permission?(user) if user.present? #<= この例外処理はGuard Clauseで書き直せる case user.permission when 1,2,3 false else true end else true end end リファクタリング後 def invalid_permission?(user) return true if user.blank? #<= この1行で例外チェックをネストなしで完了できる case user.permission when 1,2,3 false else true end end Guard Clauseの後は1行空けておくと、後から読む人に親切です。 morimorihogeコメント 処理の実装部分の書き方ってプログラマによって独自の癖があって見ていて面白いですよね。社内でもオブジェクト指向言語的に忠実に書こうとする人もいれば、考えた順番に書く人、Ruby的にlambdaとかを使って極力コードを短く済ます人など、個性が表れる部分だと思います。 ただ、どんな書き方であっても可読性は後々大事になってくる所なので、今回のGuard Clauseみたいな話はレビューで見つけたら指摘するようにしています。ただ、あまり細かく指摘しすぎても「レビュアーの書くコードと違うからダメ」みたいになって良くないので、直すべきレベルのものと個性で済ませられるもののさじ加減は難しい所ですね(それが行き過ぎるとレビュアー以外コード書けなくなるので)。 今回のコード例で言えば「否定型のメソッド」は直感的な理解を妨げるので僕は避ける宗派です(参考:Rubyにおけるunlessとコードの読みやすさについて)。 僕が書くとすると、#invalid_permission?ではなく#valid_permission?をメソッドとして実装し、利用側では if invalid_permission?(user) # … end ではなく unless valid_permission?(user) # … end の方が頭に入りやすいと思ったりもするのですが、これは多分好みの範囲な気がします。 他にも#present?に対する#blank?の様に、#valid_permission?を実装した上で逆の意味を持つ#invalid_permission?を def invalid_permission?(user) !valid_permission?(user) end の様に宣言するなど、色々と言いたいことは出てくるのですが、これも多分好みとプロジェクトのポリシ次第な感じがします。 エンジニアにとって自分で考えて書いたソースコードというのは大なり小なりこだわりがあるものなので、レビューするときには「これは誰が見ても直した方が良い部分なのか?」を気にしつつコメントするようにしています。どんなこだわりがあろうとダメなコードをダメと言えないチームは腐っていると思うのですが、指摘するときには「クソコードを憎んでプログラマを憎まず」スタンスでありたいものです。 merge/pull request reviewベース開発を進めていく中では「言いたいことは言うが、お互いを仲間として尊重し合う」という空気感が大事だと思いますので、こういう言い方やニュアンスというのも少しは気にしていくと、良い開発チームになっていくのではないかと思いました。 関連記事 Rubyにおけるunlessとコードの読みやすさについて [Ruby/Rails] strftimeのよく使うテンプレート [Ruby/Rails] strptimeのよく使うテンプレート