should_send_email という変数名は適切か？

ブール値を返す関数、もしくは変数で、`is_xxx`, `should_xxx`などの命名が使われることがあります。
しかし、とあるコードを見た時にモヤっと感じたので、命名に関する自分の考えをまとめたいと思います。

この記事は、Rubyを使ってサンプルコードを書いていますが、固有の機能は極力使わず、言語一般レベルの議論ができればと思います。  
(※Rubyにはメソッド名に「?」を使うことができ、慣習的にブール値を返します。この記法だけ今回は使用します。)

# 変数が何の情報も与えていない場合

具体的には、下記のようなコードです。
```ruby
should_send_email = user.subscribe_mail_magazine? && mail_magazine
if should_send_email
  send_email(user.email, mail_magazine.content)
end
```

テキトウに読み下すと、「emailを送るべきとき（ifの条件）、emailを送る(ifのブロック)」と読めると思いますが、この変数は果たして必要なのでしょうか？

emailを送るべき場合にemailを送る、というのは当たり前過ぎて、冗長に感じます。
条件の部分が何も情報を加えていないからです。

これならば最初から、変数を定義せずに下記のように書いたほうが、「ユーザーがメルマガを登録していて」かつ`mail_magazin`が空でない時、メールを送る、と読むことができ、条件＋実行部分の役割分担がしっかりとした構造になりスマートではないでしょうか。
```ruby
if user.subscribe_mail_magazine? && mail_magazine
  send_email(user.email, mail_magazine.content)
end
```

では、いくつかの条件（ブール値）の組み合わせを変数にまとめるのは無意味なのでしょうか？

# 条件を変数でまとめるほうが良い場合

当然ですが、業務コードは次第に書き足されていくものなので、以下のように条件が増えていくかもしれません。
```ruby
if user.subscribe_mail_magazine? && mail_magazine && user.accept_genre(mail_magazine.genre) && user.last_sign_in_at > Date.today - 30
  send_email(user.email, mail_magazine.content)
end
```

このように数珠つなぎでどんどん条件が付け足されていくと、意味のまとまりが掴みづらくなります。

ユーザーがメルマガを許容しているかどうか、ということを一つの変数名として、条件式を分割してみます。
```ruby
user_allow_mail_magazine = user.subscribe_mail_magazine? && mail_magazine && user.accept_genre(mail_magazine.genre)
if user_allow_mail_magazine && user.last_sign_in_at > Date.today - 30
  send_email(user.email, mail_magazine.content)
end
```
(Rubyユーザーならぼっち演算子を使うべきという気持ちになりますが、言語一般レベルのお話がしたいので、あえて使ってません。)

今後、メールマガジンの受け取りに関して設定項目が増えた場合は
`user_allow_mail_magazine`変数に条件式を付け足し、その他は if文の末尾に付け足せば良い、ということが明示できるようになります。

これは関数分割にも似ていると思います。

上記のコードではまだ、本当に変数に条件を分割すべきなのか？というのがまだ納得できないかもしれせん。
以下はどうでしょうか？
```ruby
if !user.deleted_at && user.last_sign_in_at > Date.today - 60 \
  && (user.subscribe_mail_magazine || user.accept_genre(mail_magazine.genre)) \
  && !mail_magazine.content && mail_magazine.status == 'ongoing'
  send_email(user.email, mail_magazine.content)
end
```
どこに意味のまとまりがあるかわかりません。また、`||`と`&&`が複雑に用いられていてメンテナンス性も非常に悪いです。

意味のまとまりから、以下のように改善できます。
今後、例えばユーザーがアクティブかどうか？という条件に何か追加されたとしても、容易に追加できます。
```ruby
user_active = !user.deleted_at && user.last_sign_in_at > Date.today - 60
user_allow_mail_magazine = user.subscribe_mail_magazine || user.accept_genre(mail_magazine.genre)
mail_magazine_active = !mail_magazine.content && mail_magazine.status == 'ongoing'

if user_active && user_allow_mail_magazine && mail_magazine_active
  send_email(user.email, mail_magazine.content)
end
```

ここまで読んでいただければわかるかと思いますが、`should_send_email`という変数名では変数名が意味する役割が大きすぎて、条件式の分割が難しいということがわかるかと思います。

※ただし、条件式を再利用する場合、または`should`が新しく何か情報を加える場合など、以下のように有効な場合もあります。(こういう設計が良いかどうかとは別として)
```ruby
should_send_email = user.subscribe_mail_magazine? && mail_magazine

if should_send_email
  send_email(user.email, mail_magazine.content)
end

...

# emailを送るべき場合は、メール送信者リストにも追加する
if should_send_email
  mailed_list.push(user)
end
```
これは`should_send_email`の条件式が何か追加された場合でも対応できます。
また、自明でない情報を追加しているということで、`should`が意味を持っています。

# まとめ
条件が3つ、4つと増えたり、`&&`, `||` で複雑になり始めたら、条件を変数に分割することを一考に入れてみてもよいでしょう。

また、まだまだ浅学の身ですので、これは変数名が適切でない、こういう場合は変数に分割しないほうが良い、などなど何か意見ございましたら、教えていただけると幸いです。

# 参考文献
https://buildersbox.corp-sansan.com/entry/2020/05/29/110000#:~:text=%E3%82%89%E3%82%8C%E3%81%BE%E3%81%99%E3%80%82-,%E4%BA%8B%E4%BE%8B%3A%20%E8%A4%87%E9%9B%91%E3%81%AA%E6%9D%A1%E4%BB%B6%E5%BC%8F,-function%20isPubliclyValid()
こちらで書かれていたレビュー指摘点の記述も参考にさせていただきました。

https://www.oreilly.co.jp/books/9784873115658/
リーダブルコード