ぼくのかんがえた最強のリーダブルコード

こんにちは、おのぽんです。

最近めちゃくちゃ涼しくなってきて、そろそろ冬用のコート出さないといけないなという気持ちになっています。

久々にQiitaの記事を書きたくなったので書いていこうと思います。
僕の現職ではPHPをメインに活用しているので、コード例はPHPで表現していこうと思いますが、
プログラミング全般のお話をしていくつもりなので、PHPがわからなくても理解できるように書いていこうと思います。

リーダブルコード書けていますか？

早速本題です。
エンジニアであるあなたは、リーダブルコードを書けていますか？
書けている自信のある方は、本記事を読む必要はないと思います。
少しでも自信のない方は、ぜひご一読いただけると嬉しいです。

本記事の対象者

• リーダブルコードの判別がつかない方
• リーダブルコードの本を読む時間はないけど、書けるようになりたい方
• コードレビューをする際、もっと良い書き方がある気がするけどうまく伝えることができない方

本記事で触れないこと

• コードの汎用性や拡張性
• オライリーのリーダブルコードの詳しい内容（似たような内容を別の表現方法で表現している可能性はあります）
• 変数名やメソッド名の命名の基本（メソッド名は動詞から始めるだったり、booleanはisやenableなどを利用しようだったり）

リーダブルコードとは

オライリーが出しているリーダブルコードが答えだと思います。
なので、この本読むのが多分一番の近道と考えます。

ですが、1冊を読んで理解して実践まで移せるようになるのって、このような1記事を読むよりも労力がかかります。

なので、本記事ではその辺りをもっとよりカジュアルに捉えていきながら、どのようなコードがリーダブルコードと言えるか？という点に迫っていきます。

ぼくの考えた最強のリーダブルコード

異論は色々あるかと思いますが、僕にとっては、

周りのエンジニアや未来の自分が理解できるコードとなっているか

という点が満たせていれば、それはリーダブルコードであると考えます。

クリエイターが複数いる組織の中で開発を行う際、おそらくコードレビューをする文化も存在していることと思います。
組織で開発を行う以上、自分1人が理解できるようなソースコードは求められていません。
極論「もし自分が明日死んでも運用できるソースコード」を書く必要があります。いわゆる属人化しないように心がけるというものですね。

そのため、周りのエンジニアが理解できる状況を作っていく必要があります。

　僕がリーダブルコードを書く上で常に意識していることは、周りのエンジニアに「未来の自分を含む」ということです。
僕は物覚えが悪いので、数ヶ月後には自分の書いたコードを忘れてしまいます。
しかし、書かれたコードの意味を読み解くことはできるので、自分の書いたコードを利用したり意図を思い出す作業にさほど時間を費やすことはありません。

特別思い出しやすい工夫をしているわけでもありません。
僕はソースコードに不必要にコメントを残すべきではないと考えているので、ほとんどコメントも残しません。

そんな状態であっても数ヶ月前に書いたコードをぱっと理解しなおせたり、チーム内でコードレビューを依頼した際「LGTM」をもらえるということは、僕を含む周りのエンジニアにとってそのコードがリーダブルコードとなっているからであると考えます。

最強のリーダブルコードであるかどうかの判断基準

ここからは、どんなコードが最強のリーダブルコードであると判断できるかの基準を説明していきます。
基準はとてもシンプルです。
メソッド名や変数名、ロジックを見た時、頭に？マークが浮かばなければ、それはリーダブルコードです。
逆に言うと、少しでも疑問を感じるコードは、最強のリーダブルコードとは言えません。

ぼくのかんがえた最強のリーダブルコードへの道

例えば、下記のようなコードがあったとします。
なお、下記のコードのロジックはすべて正とします。

class Human
{
    protected $age;
    
    public function __construct($age)
    {
        $this->age = $age;
    }

    // $drinkを飲むことができるかチェックする
    public function check($drink)
    {
        if ($drink->getVolume() > 0) {
            if ($this->age >= 20) {
                return true;
            } else {
                if ($drink->getAlcholPercentage() > 0) {
                    return false;
                }
            }
        } else {
            return false;
        }
    }

    public function drinkAll($drink)
    {
        $drink->setVolume(0);
    }
}

class Drink
{
    protected $alcholPercentage;
    protected $volume;

    public function __construct($alcholPercentage)
    {
        $this->alcholPercentage = $alcholPercentage;
        $this->volume = 100;
    }

    public function getAlcholPercentage()
    {
        return $this->alcholPercentage;
    }

    public function getVolume()
    {
        return $this->volume;
    }

    public function setVolume($volume)
    {
        $this->volume = $volume;
    }
}

$human = new Human(age: 18);
$coke = new Drink(alcholPercentage: 0);
$coke->setVolume(alcholPercentage: 0);
$coffee = new Drink(alcholPercentage: 0);
$beer = new Drink(alcholPercentage: 5);
if ($human->check($coke)) {
    $human->drinkAll($coke);
}

if ($human->check($coffee)) {
    $human->drinkAll($coffee);
}

if ($human->check($beer)) {
    $human->drinkAll($beer);
}

少し長くなりましたが、喉の乾いた青年が飲み物を飲もうとしているようです。

長くなったとは言えど、このコードは数十行程度なので読むことはできるかと思いますが、決して読みやすい状態ではありません。

早速このコードをリーダブルコードへと改修していきましょう。

？マークの見つけ方

その1： コメントに頼らずとも、メソッド名と引数からロジックを想像できるかどうかを考える

先ほどの例の1つ目のメソッドを見てみましょう。

// $drinkを飲むことができるかチェックする
public function check($drink)
{
    if ($drink->getVolume() > 0) {
        if ($this->age >= 20) {
            return true;
        } else {
            if ($drink->getAlcholPercentage() > 0) {
                return false;
            }
        }
    } else {
        return false;
    }
}

と書いてありました。

このメソッド名と引数にだけ着目してみましょう。

public function check($drink)

どうやらドリンクをチェックしているようですね。
しかし、ドリンクの「何」をチェックしようとしているのでしょうか？
コメントを読めば、 // $drinkを飲むことができるかチェックする ことがわかりますが、
このメソッド名だけでは想像することができません。
なので、このタイミングで「おや？」っと思うことができます。

ではどのように直していくべきでしょうか。

基本的にこのような補足コメントを残さなければならないコードはリファクタリングの余地があるコードであると考えた方が良いです。
全てのコメントが不要であるとは思いませんが、不必要にコメントを残してしまうと逆に運用しづらくなる可能性があります。
例えば補足コメントの対象のロジックを改修した際、補足コメントをアップデートし忘れてしまうと、一体何が正しいのかがわからなくなってしまいます。
なので、コメントを書かなくても理解できるコーディングを心がけていきたいと考えます。

今回はコメントの通り「 $drink を飲むことができるかチェックする」ことを表すメソッド名にすれば良いので、drinkable とするのはいかがでしょうか？

public function drinkable($drink)

対象の $drink が飲めるかどうかを判定するロジックであることが想像しやすくなりました。
このメソッド名でしたら、 // $drinkを飲むことができるかチェックする とわざわざ書かなくても理解できるかと思います。

その2： 複雑な分岐を作っていないか

先ほどのdrinkableメソッドのロジックを見てみましょう。

if ($drink->getVolume() > 0) {
    if ($this->age >= 20) {
        return true;
    } else {
        if ($drink->getAlcholPercentage() > 0) {
            return false;
        } else {
            return true;
        }
    }
} else {
    return false;
}

if/elseはプログラムを書く上で必要不可欠な構文です。
しかし、ネスト（入れ語。ここではif文の中でさらにif文が存在することを指しています）が深くなってしまうと、理解するのに時間を要し、最終的に「何が言いたいんだっけ？」と頭にはてなマークが浮かぶこととなるでしょう。
今回は、これらのロジック全般を見直していきます。

まず、 if $drink->getVolume() > 0 から着目していきます。
飲み物が入っている時に、さらにいろんな条件で飲むことができるかどうかを判定していこうとしていますが、
飲み物が入っていなければ、drinkableはfalseとなるので、さっさとリターンしてしまいましょう。（アーリーリターンと呼びますが、リーダブルコードを意識しながらコーディングすることができれば、このようなテクニックを知らなくても表現することはできると思います。）

if ($drink->getVolume() === 0) {
    return false;
}
if ($this->age >= 20) {
    return true;
} else {
    if ($drink->getAlcholPercentage() > 0) {
        return false;
    } else {
        return true;
    }
}

これでネストを一つ減らすことができました。

続いて、 if ($this->age >= 20) に着目しましょう。
こちらも、20歳以上であればアルコールの有無に関係なく飲むことができるので、アーリーリターンしてしまいましょう。

if ($drink->getVolume() === 0) {
    return false;
}
if ($this->age >= 20) {
    return true;
}
if ($drink->getAlcholPercentage() > 0) {
    return false;
} else {
    return true;
}

ネストがとても少なくなりました。

最後に残るは $drink->getAlcholPercentage() > 0 です。
ここでは20歳未満が対象となるので、

• アルコールが入ってなければ true
• 入っていればfalse

を返すだけでよくなります。
ので、 return $drink->getAlcholPercentage() === 0; と表現することができます。

public function drinkable($drink)
{
    if ($drink->getVolume() === 0) {
        return false;
    }
    if ($this->age >= 20) {
        return true;
    }
    return $drink->getAlcholPercentage() === 0;
}

drinkableメソッドがだいぶスッキリしました。
しかし、まだ直感的ではありません。

その3： 要約できそうな条件は存在するか

先ほどのdrinkableメソッドを見てみましょう。
条件は3つほど存在しています。

① $drink->getVolume() === 0
② $this->age >= 20
③ $drink->getAlcholPercentage() === 0

このままでも読めなくはないのですが、「つまり何を伝えたいの？」という部分を詰めていくと、

① 飲み物が空であるかどうか
② 大人であるか
③ アルコールは含まれていないか

であることがわかります。

あとはこれらをメソッド化してあげるとわかりやすくなります。（下記コードはポイントだけ抜粋）

class Drink
{
    public function isEmpty()
    {
        return $this->volume === 0;
    }

    public function hasAlchol()
    {
        return $drink->getAlcholPercentage() === 0;
    }
}

class Human
    public function isAdult()
    {
        return $this->age >= 20;
    }

    public function drinkable($drink)
    {
        if ($drink->isEmpty()) {
            return false;
        }
        if ($this->isAdult()) {
            return true;
        }
        return !$drink->hasAlchol();
    }
}

だいぶリーダブルになってきました。

その4: ロジック内に否定系は存在するか

否定系も重なるとよくわからなくなります。
例えば、下記のコードは一体どういうことでしょう？

$isNotDistiny = false;
if (!$isNotDistiny) print("This is a distiny.");

これは結局・・・運命なのでしょうか？
「運命感じなくなくない？」みたいな感じで話しかけられても、「結局どっちやねん」となってしまいますよね。
そういったコーディングは混乱の種になりかねます。

先ほどのコードでも、

return !$drink->hasAlchol();

「アルコールが含まれていないかどうか」となってしまっているので、肯定系のメソッドに変えてあげましょう。
「アルコールが含まれていない = ソフトドリンクである」なので、 isSoftDrink メソッドを作っていきます。

class Drink
{
    public function isSoftDrink()
    {
        return !$this->hasAlchol();
    }
}

class Human
    public function drinkable($drink)
    {
        if ($drink->isEmpty()) {
            return false;
        }
        if ($this->isAdult()) {
            return true;
        }
        return $drink->isSoftDrink();
    }
}

否定系はメソッド内で1つだけ書いてある文には分かりやすくなることがあります。
今回で言うと、ソフトドリンクの反対はアルコール飲料

その5: 分岐を繋げても違和感がないか

分岐の肯定 or 否定系を統一できれば、なんとなく同じ条件となりそうな部分も見えてきます。

drinkableメソッド内のロジックで、

if ($this->isAdult()) {
    return true;
}
return $drink->isSoftDrink();

この二つは肯定系のメソッド名となっているので、繋げても混乱は少ない可能性があります。
ただ、不用意に繋げすぎても逆に理解しづらくなる可能性があるので、試しに繋いでみましょう。

return $this->isAdult() || $drink->isSoftDrink();

大人であるかソフトドリンクであればtrueという状況を作れました。
この辺は好みになってくるかと思いますが、僕的にはこれくらいはまとまってても全然理解しやすいコーディングだと思うので、改修してしまいましょう。

public function drinkable($drink)
{
    if ($drink->isEmpty()) {
        return false;
    }

    return $this->isAdult() || $drink->isSoftDrink();
}

drinkableメソッドがとてもシンプルな実装になりましたね！

その6: 簡潔に述べすぎてしまっているメソッドはないか

HumanクラスのもつdrinkAllメソッドのロジック内はこのようになっています。

public function drinkAll($drink)
{
    $drink->setVolume(0);
}

全て飲み干すことを表現するように、 $drink->setVolume(0); が実装されていますね。
setterをそのまま書いても伝わりはするのですが、「ドリンクの量を0にする」という表現は日々の生活の中で目にする機会はほとんどありませんよね？
「コップの中身を空にする」などとは言いますが。
と言うわけで、この辺りの表現を直感的に理解できるように「空にする」というのをメソッド名で考えましょう。
emptyという英語が「空にする」という動詞ではありますが、飲み物が勝手に空になることはあまり想像しづらいので、どちらかと言うとemptyを名詞として使い、becomeEmptyと表現した方が自然なように見えます。
今回はbecomeEmptyを採用しようと思います。

class Drink
{
    public function becomeEmpty()
    {
        $this->setVolume(0);
    }
}

class Human
{
    public function drinkAll($drink)
    {
        $drink->becomeEmpty();
    }
}

その7: 単位は明瞭かどうか

先ほどの見解と矛盾してしまいますが、単位はコメントを残しておくとわかりやすくなる場合があります。

例えば、

const DISTANCE = 100;

と書かれていても、100cmなのか100mなのか・・・はたまた100kmなのかは実装を見てみないと分からないのですが、

const DISTANCE = 100; // cm

と書いてあれば100cmを表していることが容易に分かります。

今回のコード例では、

class Drink
{
    protected $volume;
}

この単位が不明瞭でした。

そのため、単位をコメントしておきます。

class Drink
{
    protected $volume; // ml
}

これだけでグッと意味のわかるコードになります。

---

リーダブルコードへと改修した結果

ここまでの改修により全体のコードはこんな感じになりました！

class Human
{
    protected $age;

    public function __construct($age)
    {
        $this->age = $age;
    }

    public function isAdult()
    {
        return $this->age >= 20;
    }

    public function drinkable($drink)
    {
        if ($drink->isEmpty()) {
            return false;
        }
        
        return $this->isAdult() || $drink->isSoftDrink();
    }

    public function drinkAll($drink)
    {
        $drink->becomeEmpty();
    }
}

class Drink
{
    protected $alcholPercentage;
    protected $volume; // ml

    public function __construct($alcholPercentage)
    {
        $this->alcholPercentage = $alcholPercentage;
        $this->volume = 100;
    }

    public function getAlcholPercentage()
    {
        return $this->alcholPercentage;
    }

    public function getVolume()
    {
        return $this->volume;
    }

    public function setVolume($volume)
    {
        $this->volume = $volume;
    }

    public function isEmpty()
    {
        return $this->volume === 0;
    }

    public function hasAlchol()
    {
        return $drink->getAlcholPercentage() === 0;
    }

    public function isSoftDrink()
    {
        return !$this->hasAlchol();
    }

    public function becomeEmpty()
    {
        $this->setVolume(0);
    }
}

$human = new Human(age: 18);
$coke = new Drink(alcholPercentage: 0);
$coke->setVolume(alcholPercentage: 0);
$coffee = new Drink(alcholPercentage: 0);
$beer = new Drink(alcholPercentage: 5);
if ($human->drinkable($coke)) {
    $human->drinkAll($coke);
}

if ($human->drinkable($coffee)) {
    $human->drinkAll($coffee);
}

if ($human->drinkable($beer)) {
    $human->drinkAll($beer);
}

いかがでしょうか？
各メソッドのロジックが全てシンプルな形となり、またコメントがなくてもぱっと見で理解できるコードにすることができました。

今回の例のコードは最初から複雑性は少なかったので、そこまで苦労せずリーダブルコードにすることができましたが、
実際のコードにおいても、上記のように「？マークを見つける」ことを見つけられれば、リーダブルコードへの糸口を掴むことができます。

ぼくのかんがえた最強のリーダブルコードを書く上で大事なポイントをまとめると、

ゴール： 周りのエンジニアや未来の自分が理解できるコードを書くこと
そのためには： メソッド名や変数名、ロジックを見た時、頭に？マークが浮かぶかどうかを見極める

本記事では、?マークの見つけ方を7つ紹介しました。
1： コメントに頼らずとも、メソッド名と引数からロジックを想像できるかどうかを考える
2: 複雑な分岐を作っていないか
3: 要約できそうな条件は存在するか
4: ロジック内に否定系は存在するか
5: 分岐を繋げても違和感ないか
6: 簡潔に述べすぎてしまっているメソッドはないか
7: 単位は明瞭かどうか

これら全てを網羅する必要はないとは思いますが、これらを満たしていくと、リーダブルコードにグッと近づくことと思います！
参考にしていただけると嬉しいです☀️

最後までご一読いただきありがとうございました！

謝辞

@myr 様

ご指摘いただきありがとうございます！！✨
おかげさまで、より最強のリーダブルコードに近づきました！！