こんにちは。新卒1年目エンジニアのKFです！
学生時代は、SwiftやRubyを書いたり、機械学習を触ったりしていました。現在は主にPHPを書いています。
今回は、配属されてから1ヶ月ちょっとの間でたくさんのコードレビューを頂いたので、その中でも主に命名規則について共有したいと思います。

背景

そもそもなぜこのテーマなのか

私は学生のころにコードを書いていましたが、コードレビューしたこともされたこともありませんでした。そのため、「自分が分かればいいや！」という感じで書いていて、命名規則などがバラバラになっていました。今見返してみると何書いているかわかりません。笑

こんな全く命名規則などを気にしてなかった私がどのような点に気をつけるようになったか、実際にどのようなコードレビューを貰ってどう改善したかを共有できればと思います。

今どんなことやっているの？

現在は社内の営業の方たちが使用する「レバ☆スタ」と呼ばれるWebシステムの開発を行っており、CakePHPを利用して開発しています。以下のように登録されている求職者の方達の情報を管理しています。

命名規則について

コードレビューをいただく中で、「自分が書いたコードを他の人が見ても理解できること」「未来の自分が見ても理解できること」という2点を意識してコードを書くようになりました。

ここでは私が業務中にコードレビューしてもらって、どのような指摘を受けたか、そしてどのように修正したかを紹介できればなと思います。ただ、命名規則はチームによって違ったりするのであくまで参考程度でお願いします。また、コーディング規約などはPSR-1やPSR-2も参考にしてみてください。

1. 名前は分かりやすく長くならないように

メソッド名をつける時に分かりやすさを意識しすぎて以下のように長くなってしまいました。

修正前

private function getDaysFromOfferDateToInterviewDate($employee_id){
    //・・・・ 中略 ・・・・
}

コードレビューの際に「なんかメソッド名違うしそもそも長いかな」とコメントをいただきました。面談日から内定日までにかかった日付を返すメソッドだったのですが、今見返すと自分でも長いなと思ってしまう..... なので以下のような改善をしました。

修正後

private function getDaysFromInterviewDate($employee_id){
    //・・・・ 中略 ・・・・
}

修正前のメソッド名だと長すぎてどういうメソッドなのか理解するのに時間がかかりますが、修正後のメソッド名だと名前が短くても意味が理解できるようになってるかと思います。

2. 変数名が何を指しているのか明確にする

ユーザの情報をDBから取ってきて、それを格納する変数名をこのようにしていました。

修正前

$userName = $this->MsEmployee->find('first', array(
        //・・・・ 中略 ・・・・
    )
);
return $userName;

ここで指摘されたこととして「ユーザ名以外も取得しているのでuserNameという変数名はおかしい」というものです。

実はユーザ名だけを取得しているつもりで命名したのですが、実際は他の情報まで取得していました。変数名を正しくつけなければ間違って使ってしまう恐れがあります。

これを踏まえて以下のように修正を行いました。

修正後

$employee_user = $this->MsEmployee->find('first', array(
        //・・・・ 中略 ・・・・
    )
);
return $employee_user;

修正前の変数名の場合、格納されているものと変数名が違うので、実際に変数を利用しようとした時に意図しないことが起こる可能性があります。修正後の場合、正しい変数名になっているので意図しないことが起こりにくくなります。

なので変数名は正しく分かりやすいものを付けましょう！

3. メソッド名は正確に

あるかないかをチェックするメソッドを書いていた時に、先頭にcheckをつけて命名していました。

修正前

public function checkEmployeeOffers(){
    //・・・・ 中略 ・・・・
    return true;
}

ここでは「checkは便利な言葉だけど何をチェックしたいのかが分からないからisやhasを使いましょう。」とレビューを頂きました。 "存在する"ことをチェックしたいのか"存在しない"ことをチェックしたいのかが人によって捉え方が違うのでcheckという単語は便利だけど使わないように、ということでした。よってcheckを使わずにisを使うこととしました。

修正後

public function isOffers(){
    //・・・・ 中略 ・・・・
    return true;
}

これによって誰がこのメソッドを見ても"存在する"ことをチェックしたいんだなと明確になります。

その後に「存在するか」というものには先頭にisをつけるようになったのですが、ある時に以下のようにメソッド名を命名しました。

修正前

public function isEditable(){
    
    //・・・・ 中略 ・・・・
        
    return json_encode($business_trip_contents);
}

ここでは「return true/falseであればisでいいけど何かを返しているからメソッド名は適切ではない」と指摘を受けました。私の中では「存在するか」をチェックしているのでisをつけていたのですが、その場合はtrue/falseを返さなければいけないのにjsonを返していました。この場合はisではなくgetをつけるべきなので以下のように修正をしました。

修正後

public function getBusinessTripContents(){
    
    //・・・・ 中略 ・・・・
        
    return json_encode($business_trip_contents);
}

これによってtrue/false以外のものが返り値として設定されていることを理解することができます。

まとめ

配属されて1ヶ月ほどですが、命名規則以外のことでも丁寧にコードレビューをして頂いているので学生の頃にはなかった経験を日々しています。その中でも自分の書いたコードを他の人が見てもすぐに理解できるように書くことは難しいなと実感しながらコードを書いています。特に命名規則などをあまり気にせずに書いてる人は少しずつでいいので意識するべきだと思います。

そういう人たちはとりあえず「リーダブルコード」の本を読むことがおすすめです！私もまだまだなので早くコーディング技術を上げていきたいと思います！