???「お前のコードのせいでプロダクトが死んでいる」
この間レビュー中にふと思いついたネタでした。
懐かしいですね、TVアニメが放送されたのは2010年なので13年前ですね。
時の流れは残酷ですね。
はじめに
今回は命名について話していきます。
レビューをしていると、自分自身が良くない命名をしてしまったり、良くない命名を見かけることがあります。
良くない命名はレビューや仕様の把握、コードの変更に良くない影響を与えると思っています。
今回は個人的に良くないと思っている命名をいくつか挙げていきたいと思っています。
(プロダクトが死ぬは流石に言い過ぎですが)
「短すぎん？」
$a;
こんな命名してたら怒られちゃうと思います。
ループカウンタや一時的な計算結果などに使われる場合は許されるかもしれませんが、数文字くらい書いたって損はないと思います。
命名を行うときは、使用目的や文脈に依存せず、見ただけでわかるような名前をつけることが大事です。
「構造の名前つける必要ある？」
$dictionary;
$userMap;
arrayって書かなければ良いみたいなノリ。
dictionary とか map とかその言語になくても型や構造をそのまま命名にするのは良くないです。
$users;
ユーザーを要素に持つ配列であれば↑で良いですね。
ただ、mapとか言ってるのは基本的に連想配列を示すことが多いと思います。
連想配列の命名はある程度パターン化した方が命名に迷わなくて良いです。
$useIdToUserName;
自分はkeyToValueパターンを好んで使用しています。
命名を行うときは、構造や型を明示するよりも、"keyToValue"のような形式を採用し、意図が直感的に理解しやすい名前を選ぶことが大事です。
「一貫性がないよね」
$createUserService;
$userCreationService;
正直どちらが良いとかはないです。
プロジェクト内で既に存在する命名を確認した上で一貫性を持たせることが大事だと思います。
一貫性が失われてくると無法地帯になると思っています。
1つのリポジトリ内で、xxxService、xxxInteractor、xxxCreationのようなフリースタイル俺俺設計が闊歩するようになると辛いです。
命名を行うときは、プロジェクトの既存の命名規則に照らし合わせ、一貫性を保った名前をつけることが大事です。
「ほんとにその単語でいいの？」
$right;
例えば「権利」を表現するとき調べてすぐ出てくる単語は、rightになることが多いのかなと思っています。
ただ、「権利」を表す単語は細かいニュアンスの違いで複数の単語が存在します。
例としていくつか挙げると以下のような感じです。
entitlement（人が行うまたは受け取る）
privilege《コ》（アクセス・処理実行などの）
right（法律・伝統・自然に基づく）
命名を行うとき、日本語を元に翻訳機能を使用して英単語を決めることは割と多いと思っています。
一番最初に出てきた単語に安易に飛びついてしまうと、あまり良くない命名になってしまう可能性があります。
命名を行うときは、プロジェクトの文脈を深く理解し、関連する類語を検討した上で、そのプロジェクトに最も適した明確で記憶に残りやすい名前を選定することが大事です。
「文法あってる？」
$isEnable;
そもそも文法があっているか問題ですね。
enableは他動詞なので、そもそも文法的におかしいと言う話ですね。
boolean 命名とかで出てくるので詳細は詳細は省きますが、 is + 形容詞やis + 名詞が適切な表現です。
$isEnabled;
enableをenabledと過去分詞で表現することで、自然な命名になります。
過去分詞は形容詞的に使用ができるので、このような命名であれば文法的にも正しいと言えます。
個人的にはisEnabledだけだと何が有効化されているかわからないので、主語を明らかにするか、インスタンスのメソッド呼び出しとして表現する方が好みです。
// 主語を明示的にする
$loggingIsEnabled;
// インスタンスのメソッドとして表現する
$log->isEnabled();
命名を行うときは、基本的な英文法を遵守し、文法的に誤った命名を避けることが大事です。
「よくある形式ってもんがあるんじゃない？」
$user->deletion_date;
良くある形式を知ることが、良い命名の近道だと思っています。
laravelだとマイグレーションファイルで簡単に作成日と更新日のカラムが生成されます。
形式は以下のような形になっています。
$user->created_at;
$user->updated_at;
deletion_dateで表現したい内容が、「削除された日時」であれば形式を揃えた方がわかりやすいです。
削除予定日など「未来の日時」を表現する場合はdeleted_atを使用することが正しいとは言えません。
その場合はwill_be_deleted_atなど、うまくアレンジをして使用する必要があります。
$user->deleted_at;
良くある形式を知らずに命名をしてしまうと「一貫性のない命名」となってしまう可能性が高いです。
同じようなものは同じように扱うことの大事さは、プリンシプルオブプログラミングの中の「同型原理」でも書かれています。
私は命名でも同じことが言えると考えています。
同型原理とは、「形にこだわる」という原理。
同じことは、同じように扱うことにこだわる。
例えば、あるモジュールにおいて、そこで扱う数値の単位が統一されていたり、公開関数の引数の数が統一されていたり、使用順序が統一されていたりすること。
命名を行うときは、一般的な形式を採用し、一貫性を保つことが大事です。
「説明的すぎない？というか本当にわかりやすいか？」
$isUserActiveAndCanDeleteArticle;
まだわかりやすい方なので、これくらいでは問題になりません。
このような変数名が、テンプレートなどで使用されていることが割とあるのかなと思っています。
@if ($isUserActiveAndCanDeleteArticle)
// 削除への導線
@endif
こう言うときは、ユーザーインスタンスをもとにしたメソッド呼び出しに変更したり、その変数で何を制御したいかを考えるとスッキリすることがあります。
// インスタンスをもとにメソッド呼び出し
@if ($user->isActive() && $user->canDelete($articleId))
// 削除への導線
@endif
// 変数で何を制御したいかを表現する
@if ($isDeleteButtonVisible)
// 削除への導線
@endif
変数名が長くなればなるほど、その変数に詰め込まれている情報が増えていることを意味しています。
そもそもの設計を見直したり、視点を変えることであまり長くなりすぎないように心がけましょう。
命名を行うときは、単に条件を直接的に説明するだけではなく、より効果的なメソッド呼び出しや、問題を捉える新しい視点を取り入れることで、変数名をより理解しやすく、簡潔にすることが大切です。
「入れ替わってない？」
$article = Article::find(1);
$article = new ArticleResource($article);
同じ変数への再代入です。
1行目では、ORMから取得したインスタンス(Eloquentモデル)を、2行目ではEloquentモデルをもとに生成したリソースクラスを代入してます。
リソースクラスは、Eloquentモデルと実際にアプリケーションのユーザーへ返すJSONレスポンスの間に変換レイヤーです
このようにごく短いスコープでは問題ないですが、再代入はあまり推奨されるものではありません。
ある時を境に中身が変わってしまうからです。
$articleModel = Article::find(1);
$article = new ArticleResource($article);
このように適切にわかりやすい命名を行うことが推奨されます。
再代入 何が悪いとかで調べてもらうと出てくるので説明は省略します。
命名を行うときは、変数の再代入を最小限に抑え、明確な定義を心がけることが大事です。
まとめ
時間は有限なので、命名に時間をかけすぎるのは良くないでしょう。
だからと言って適当な命名をしても良いことにはならないので、普段から命名を意識することで少しずつ良い命名ができるようになると良いのではないかなと思います。
わかりやすい命名を心がけていても、たまに命名に困ることがあったりします。
そのような場合は、クラスや変数が多くの役割を持ちすぎてしまっていることが多いです。
命名を意識していると、このような設計の誤りにも気づきやすくなります。
命名の大切さを書いてきましたが、極端に短いスコープや、一回しか使わない処理に時間をかけるのは労力に見合わないこともあるのでバランスが大事かなと思います。
ただ命名がしっかりとしているコードはレビューがしやすいのですぐに見てもらえたりします。
その為、結果としては適当な命名を行なったコードよりもマージまでの時間も短くなるのではないかなと思っています。
レビューがボトルネックになって作業が進まない人は、これを機に命名に気をつけてみてはいかがでしょうか？
こういう命名がありましたみたいなのがあったらコメントください。