読みやすいコードを書くために、まず名前から見直そう

こんにちは。
COUNTERWORKSアドベントカレンダー22日目も担当の疋田です。

前回はRailsの保守性についての記事を書きました。今回は、名前について書きたいと思います。(2回目はちょっと雑かも)

チーム開発をやったことのある方は、思い浮かぶかもしれませんが、他のメンバが書いたコードを読んでいて、そのメソッドが何をやっているか想像できずにいちいち全部その実装を読みに行ったり、名前から処理を想像したら全然違うことやっていて詰まったり、特殊な意味を持った0やただの文字列に遭遇して、その真意を読み解くのに時間がかかったことはないでしょうか。

私はあります。
適当な名前をつけてしまうと、他のメンバがその処理を読むとき、本当に苦労することになります。
(多分私が昔書いたコードもどこかで誰かがイライラしながら読んでいることでしょう。。)

チームのために、将来の自分のために、リーダブルなコードを書くための一歩目としてこういうことを考えると良いんじゃないのって話で、どちらかというと初心者の方向けな記事です。

実際に、読みやすいコードを書くには、名前付けだけじゃなくて、設計や実装方法なども含まれてくるのですが、そちらはある程度トレーニングや知識が必要になってきます。
でも、名前付けは意識の問題で、すぐに改善できる部分もあると思うので、（実際には名前付けにもトレーニングは必要かもですが。。）まずは名前から思いやりを持って開発をするということを進めていくのが良いのかなと思います。

なお、今回の記事では、英語の文法的な話やこういった命名規則にすべきだ的なことは書きません。また、ここで扱う名前は変数やメソッド、クラスなどの名前を想定しています。

※名前は文脈によって大きく変化します。下の例が必ずしも当てはまるわけではないので、自分の書いてるコードの文脈でより伝わると思うものを決定していくのがベターです。
※本記事の参考コードはRubyです。

コメントをしなくてもわかる名前をつける

まず、当たり前ですが、クラスやメソッド、変数にはそれが意味する適切な名前をつけるべきです。その名前をみただけで、それが何をしているか、何を意味するかがわかることがベストです。

例えば以下のようなコードがあった場合

# 1ページに掲載する数
num = 10

# 選択可能なエリア
areas = ["東京", "大阪"]

# アカウントのプロフィール情報
class AccountInfo
  # アカウントのアバター画像
  def image
  end
end

コメントがなければ

• numって何に使う数値？
• areasって何に使う配列なの？
• AccountInfoって何よ？Infoって...何の情報扱ってるクラスなの？
• imageってどんな画像を返すの？

などいろいろ分からないことばかりでそれを知るためには、それが使われている場所やそれらの実装の中身をいちいちすべて見に行かなければわかりません。コメントがあればまだましですが、コメントだらけになってもそれはそれで読みづらいし、メンテもいちいち時間かかるし、スマートでもないし、あまりいいとは思えません。
なので、名前はそれが意味するものが分かる名前をつけるべきと思います。

例えば以下のようにすることでコメントがなくても、それらが何を返したり、何に必要とされているものがある程度読み取れると思います。

display_per_page = 10
selectable_areas = ["東京", "大阪"]
class AccountProfile
  def avatar_image
  end
end

このようにそれが意味する名前をつけてあげることで、多少コードが読みやすくなりそうです。

慣習、コーディング規約に従う

次に慣習やコーディング規約に従うということについてです。
多くのプログラミング言語やフレームワークは命名規則や慣習が存在しています。また、チーム毎にこうやって書いていこうといったコーディング規約があると思います。
それらはチームとして、同じような命名規則で書いていこうといったマナーです。なぜマナーがあるかですが、それはマナーがないと、みんな好き勝手やってコードが荒れるからです。無秩序はつらいです。
人によってはキャメルケースで書き、人によってはスネークケースでかく。isActiveと書く人もいればactive?と書く人もいる。こういった場合、読み手にとっては、何か意図があるのか、他に副作用などあるかなどちょっと考えてしまいます。特にその言語などに精通している人にとっては、深く考えるかもしれません。そしてコードを確認しなければならなくなります。

それは無駄な時間であり、コードも汚くなります。チームとして共通の見解を持って、実装を進めていきましょう。（もめるのは最初だけにしてください。）

# Rubyならboolean返すときは ? つける、危険なメソッドには ! つけるなどです
class User
  def admin?
  end

  def destroy! 
  end
end

名前がはっきりしていないのは役割が単一でないから

名前を考えても浮かばない場合やふわっとした名前付けになってしまうことがある場合、やろうとしていることが単一でない可能性があります。一つのクラスやメソッドにいろいろなことをやらせようとして、名前がそれに引っ張られてしまっているかもしれません。
名前が怪しいなと思ったときは、それらが本当に単一責任になっているかを確認してみるのがいいと思います。

また、責任が増えている場合、テストを書くことで洗練される場合があります。
テストを書く中で、副作用が多く書きづらかったらそれは分割すべき場所かもしれません。それらの副作用を他のメソッドに分割することで、適切な役割と名前が決まってくることもたくさんあります。

振る舞いや暗黙のルールに名前を与える

処理を書いている中で、それに名前を与えてあげることで何を意図した振る舞いなのかをわかりやすくできるという効果があります。

if user.admin? && article.draft?
  # 更新処理
end

のようなものがあった場合、多分Adminユーザなら下書きの記事を更新できるんだろうなっていうことがわかると思いますが、上から読んでいった時に、ifの中まで読んで初めて「あ、あのifは更新できる条件なんだ」ってのがわかると思います。
それよりは

article_updatable = user.admin? && article.draft?
if article_updatable
  # 更新処理
end

のほうが、その条件が何を意味しているかがすぐに分かって読みやすくなると思います。

あるいはtypeやkindなどがあってそれが特定のときはこうする。priceが0のときはこうするなどの処理をしているときでも、それがどういうときなのかを名前を与えてあげることで、読みやすくすることができます。基本的にif分の中などで条件を書くときは、それに名前を与えてあげると良いと思います。

# これだと無料なのかそれとも開発者が他の意味をもたせてるのか不安になる
if price == 0
else
end

# 無料のときってわかりやすい
is_free = price == 0
if is_free
else
end

処理内容=名前とは限らない

たまに処理内容を表す名前にしているものが見受けられますが、名前はやっていることを表すのではなく、その目的や意図することを表すべきです。そうしないと、本当は全然意図していないのに、同じ処理だからと他の部分でもどんどん使われてしまって、本来意図していたところの変更がしずらくなってしまうようなことがあります。

class Article
  def owner?(user_id)
    owner_id == user_id
  end
end
# 例なのでこんなコード書かねーよはなしで

本当はそのユーザがその記事を更新できるかをチェックしたかったのに、その処理が偶然 owner　かどうかを判定するだけだったので、そのメソッド名にしてしまい、他の部分でもたくさんつかわれるようになってしまい変更できなくなるなどです。こんなのないでしょと思うかもですが、あるときはあります。。

class Article
  def updatable?(user_id)
    owner_id == user_id
  end
end

まあ、処理内容ではなく意図することを表現する名前がいいよねというお話です。

まとめ

名前についていろいろ書いてみましたが、言いたいことは

• 読む人の気持ちになってコードを書く
• ちゃんと単一責任を意識してコードを書く

を、考えながらやっているとわかりやすいコードが書けるようになっていくのかなと思ってます。