強い先輩にコードレビューで指摘された「命名」のまとめ

はじめに

このエントリーでは、たまたま自分より圧倒的に強い先輩エンジニアの方にコードを読んでもらう機会があり、 その時に指摘されたポイント・自分で考えたことについて記述します。

※Ruby on Railsのコードについてレビューを受けていたので、その体で説明していきます。

指摘されたこと

自分が先輩に指摘されたのは、実装そのものというよりは、 命名 に関するものが中心でした。

1.命名は仕様と一致させる

例えば、 omniauth + devise を使った外部SNS認証を実装しようとしている際に、自分はSNSの uid provider をUserモデルにAuthentication(認証) という名前でモデルを作成して保存しようとしていました。そこで先輩エンジニアに、「omniauthによって認可された情報を扱うモデルになるので、Authenticationではなく Authorization(認可) が適当ではないか」という指摘をいただきました。 認証と認可、言葉の響きは似ていますが、実際に意味するところは異なっています。 適当に命名した結果「モデルの名前とモデルのやっていること」が一致していない実装 を行ってしまいました。

もしも、自分が英語の意味を間違えたままクラス・メソッドの命名を行ってしまった場合、後から他の開発者がそのコードを利用する際に混乱することは必至です。 なんとなくで命名していくのではなく、「 この処理・仕様を表すのに、自分の命名は適切か？ 」を考えるようにしなければいけないと感じさせられました。

2.命名規則を揃える

自分はあるモデルに、クラスメソッドとして、 create_from_hoge create_by_fuga のようなメソッドを定義していました。 これら二つのメソッドは、fromとbyという命名上の違いはあるものの、動作上は同一の実装になっていました。その実装を見た先輩から、「 *似た動作を行うメソッドなのに、命名規則が異なるのは違和感がある 」と指摘をいただき、両方とも create_from... に命名を揃える修正を行いました。

似ている動作を行うメソッドの命名を揃えることによって、「 実装を読まなくてもメソッド名だけである程度同じ動作をするメソッドであることが推測できる 」というメリットがあります。逆に、似ている動作をするメソッド同士なのに命名がバラバラになっている場合、命名から動作を予測しにくくなるため、コードを読む必要が出てきてしまい、可読性が悪くなってしまいます。

特に命名を分けたい理由がない場合には、なるべく命名規則を揃えていきたいと感じる出来事でした(大規模プロジェクトだと崩壊する気もする)。

気づいたこと

Lintを入れて、テストを書いて、フレームワークに則って行った実装については、そこまで酷くなることは少ないように感じています。 フレームワークに沿わないことをビジネス上多くあるから酷くなるじゃないかとかそういうことは置いておいて、何より自分に欠けていたのは 「 他の開発者に優しい命名 」という考え方です。 正確には気をつけていたものの、他の人から客観的にコードを見ていただくことで、まだまだ足りていないということが意識できてよかったです。

この「命名」というテーマ以外にも、自分一人でコードを書いていたら、きっと気づけないバッドプラクティスとか修正点とかがまだまだある気がしているので、 仕事でもプライベートでも書いたコードはガンガン人に読んでもらうようにしたいと感じました。

おまけ

命名補助サービスなんてものもあるらしい codic - プログラマーのためのネーミング辞書

それよりは英語でググって「自分が今からしようとしている実装の命名を、英語圏の人はどうしているか」を調べたほうが力がつくかもしれません。