コードの意味的な違和感を捉える

この間ペアプロを通して実装していたコードをリファクタリングしていた時に、「そのコードは意味的にわかりやすいか？」的な話が出て、あまり深く意識できてない考え方だったので勉強になった。

どういうことかって言うと。。。

例えば、とあるwebサービスでユーザーの状態によって処理を切り替えるみたいな実装を書いてたとする。こんな感じでcase-whenで（あ、rubyで書いてます）。

class HogeUserExecuter
  user = User.find_by(name: name)

  # ユーザーの状態によって処理を切り替える
  case
  when user.is_blacklist? #=> ユーザーがブラックリストに入っているか？
    ...
  when user.is_not_confirm? #=> ユーザーが仮登録状態か？
    ...
  when user.is_confirmed? #=> ユーザーが登録完了状態か？
    ...
  else
    ...
  end    
end

case文でもif文でもそれ以外でも別に何でもいいが、ここに「ユーザーが論理削除済みの場合の処理」を追加したいとなるとcase文にwhen user.is_deleted?みたいなコードを追加するのではないかと思う（どうかな？違うかな。。。僕だけ？）。

では次に、このサービスがブログサービスだったとして「ユーザーが自身のブログ記事を書いて公開している場合の処理」をこのクラスに追加したいとする（例が良くない気がするけど、あしからず）。今回だったら、case文に追加するだろうか 🤔

このcase文の一連の処理は「ユーザの状態」から処理を分岐させていると考えることができるかと思う。しかし、今回追加する要件はブログの記事を書いて公開しているだ。ブログの状態というユーザーとは別の評価対象と考えることもできるのではないだろうか。ユーザーの状態を見て確認する処理をcaseに入れると、違和感が感じる人もいるかもしれない。

class HogeUserExecuter
  user = User.find_by(name: name)

  # ユーザーの状態によって処理を切り替える
  case
  when user.is_blacklist? #=> ユーザーがブラックリストに入っているか？
    ...
  when user.is_not_confirm? #=> ユーザーが仮登録状態
    ...
  when user.is_confirmed? #=> ユーザーが登録完了状態
    ...
  when user.blogs.public? #=> ##### ここを追加 #####
    ...
  when user.is_deleted? #=> ユーザーが論理削除済み
    ...
  else
    ...
  end    
end

ここで自分が言っている違和感というのは、ユーザーの状態を判定する意味を持つ一連の処理に、ブログの公開状態を判定するという違った意味合いの処理が混ざるということだ。このcase文は意味的に伝わりやすいのか？？人に説明するとき、「ここでユーザーの状態を見て処理分岐しています。あ、でも一部ブログの公開状態をみているケースもあります」となって、説明を受けてる側が「え？なんで？」ってならないか？？？、、、こんな感じの違和感につながる。この違和感は人によって（特に開発者の経験によって）感じ方が違うかなと思っていて、まだまだ経験の浅い自分はこれに対して特に違和感もなく、一連の処理フローに入れたほうがなんとなく流れ的に良さそうだし、とりあえずcase文に入れておけば問題ないっしょ！！！と思っていた。

この書き方が間違っている！ということを言いたいのではない。別にこの書き方は正しく動作するなら間違っていないと思うし、今回の仕様を満たす実装の一つだと思う。ただこういったコードの意味的な違和感（自分はそう呼ぶことにしている）を捉えられるかどうかが、読みやすいコードを書くヒントになるのではないかと、ペアプロで先輩エンジニアに教えてもらえた。

この違和感を解決するためには、そのケースを別箇所に切り出すとかがいいかもしれない。

class HogeUserExecuter
  user = User.find_by(name: name)

  # => 記事を公開済みかどうか？
  if user.blogs.public?
    ...
  end

  # ユーザーの状態によって処理を切り替える
  case
  when user.is_blacklist? #=> ユーザーがブラックリストに入っているか？
    ...
  when user.is_not_confirm? #=> ユーザーが仮登録状態
    ...
  when user.is_confirmed? #=> ユーザーが登録完了状態
    ...
  when user.is_deleted? #=> ユーザーが論理削除済み
    ...
  else
    ...
  end    
end

もしくは処理の順序などの理由でcase文の中で書く必要があるなら、ユーザー情報を入れてる変数名とかクラス名を変えたりして、case文の意味を変えるというのもありかもしれない（いい変更例すぐ思いつかなかった）。

これが正解だ！というのはないと思う。プロジェクトの開発基準とか開発者の考え方、好みとかも関係してくると思うし、細かいかもしれないが議論の余地はありそう。

あんまりいい例ではないけど、このコードはどういった意味を持ってるのだろう？？？どういった処理をしているのだろう？？？ということを、もっと深く考える癖をつければ、リーダブルな変数名やメソッド名をかけたりすることに繋がる気がしている。

また良い習慣を教えていただけて嬉しいと思った次第でした🧡 :kansya:

蚊帳の中の日記

ゆるく生きてます

コードの意味的な違和感を捉える