良いコードって何だろう？

こんにちは。PHONE APPLIでサーバーサイドの開発をしている福田です。
2020年の4月に新卒入社して1年が過ぎ、後輩ができました。最近では「恥ずかしい姿を見せられない！」の一心で業務に勤しんでいます。

今回は、改めて「良いコードって何だろう？」とういう点で話していきたいと思います。
背景として、こちらの本を読んだのがきっかけです。

本の内容としては、オブジェクト指向言語によるドメイン駆動設計（DDD）のモデルや設計などについて書かれているのですが、最初の方にリファクタリングも触れられていたので、改めてコードの書き方について考えてみました。

弊社のプロダクト「PHONE APPLI PEOPLE (旧:連絡とれるくん)」ではサーバサイドにKotlinを採用しているため、参考例はKotlinで記述しています。

良いコードとは？

良いコードとは何でしょうか？

• メモリの使用量やCPUの負荷が少ない
• 特殊なやり方でコードを短くする

もちろん、ゲームや特殊な環境を想定しているためメモリなどを気にしないといけないという事はあると思います。また、特殊なやり方でコードを短くしたとしても、別の人がコードなどを修正する際に理解するための学習コストがかかってしまいます。
基本的にはあまり良いコードではないと思います。

では良いコードとは何か？
私は「他の人が見て、理解しやすいコード」だと思います。

良いコードを書くために

そのために何に気をつけたら良いのか、いくつか紹介します。

変数名や関数名で用途が分かる

名前は大事です。
細かい処理の内容などが分からなくてもイメージをすることが出来ます。
良くない名前を付けた変数と良い名前を付けた変数の例を記述しました。

// 1文字
val a

// 何かの省略？それとも英単語？
val up

// 意味のある単語
val unitPrice

上記3つの変数名なら、ほとんどの方が3番目の変数の使用目的のイメージがしやすいと思います。
2つ目の変数名も「up」という単語なのか、それとも省略した文字なのかもわからないです。「up」なら数字を増やすイメージを持ちますが、もしこれが「unitPrice」の略であるなら違う用途のイメージになります。

関数を作成し独立させる

例えば以下のようなコードがあったとします。

// 数量 x 単価 の値（税抜き）
val basePrice = quantity * unitPrice
// 送料（初期値は0）
var shippingCost = 0

// 税抜き価格が3000未満のとき、送料が500増える
if (basePrice < 3000) {
     shippingCost = 500
}

// 支払い金額（税込）
val itemPrice = (basePrice + shippingCost) * 1.1

上記の場合でもコード自体は読むことは難しくないと思います。
しかし、税抜き価格が1,000未満の時、もしくは特定の商品の時など、特定の条件の時に送料が変化するパターンなどが追加されると、条件が増えるたびにelse句を追加することになります。これではメインコードが読みにくくなる可能性が出てきます。

そこで、送料を計算するロジックを関数にして独立させます。

// 数量 x 単価 の値（税抜き価格）
val basePrice = quantity * unitPrice

// 送料を返す関数
val shippingCost = shippingCost(basePrice)

// 支払い金額
val itemPrice = (basePrice + shippingCost) * 1.1

~~~~~~~~~~~~~~~~~~~~~~~~~

// 送料を返す関数
fun shippingCost(basePrice: Int): Int {
    if (basePrice < 3000) {
        return 500
    }
    return 0
}

上記の形で送料を計算する関数を独立することができました。この形であれば、例えば計算のロジックが変更されたとしても影響が関数内だけになります。他の箇所で送料の計算が必要な場合でも作成した関数にアクセスすれば新規で作成する必要はなくなります。

消費税計算のところも同じようにtaxRate()みたいな関数を用意すれば、消費税が変わったとしても修正内容はtaxRate()箇所だけで済みます。

分岐条件を単純にする

例えば、以下のようなコードは読みにくくないですか？

fun fee() {
    //  料金を格納する変数
    var result: String

    // ユーザの情報から子供ならtrueを返し、子供料金をresultに格納する
    if (isChild()) {
        result = childFee()

        // 大人ならtrueを返し、大人料金をresultに格納する
    } else if (isAdult()) {
        result = adultFee()

        // 上記以外はシニア料金をresultに格納する
    } else {
        result = seniorFee()
    }

    // 料金を返す
    return result
}

この場合、変数resultを宣言し、else句を使って子供、大人、シニアの料金を格納しています。
しかし、上記の例だと条件が決まれば料金が決定します。料金が決まった段階で、値を返してあげればresultは必要ありません。

fun fee() {
    // ユーザの情報から子供ならtrueを返し、子供料金を返す
    if (isChild()) {
        return childFee()

        // 大人ならtrueを返し、大人料金を返す
    } else if (isAdult()) {
        return adultFee()
        
        // 上記以外はシニア料金を返す
    } else {
        return seniorFee()
    }
}

余計な変数がなくなったので、少しコードが読みやすくなりました。
さらに現状ではifを使っていますが、whenを使用しreturnを外に出して1つにします。

fun fee() {
    return when {
        // ユーザの情報から子供ならtrueを返し、子供料金を返す
        isChild() -> childFee()

        // 大人ならtrueを返し、大人料金を返す
        isAdult() -> adultFee()

        // 上記以外はシニア料金を返す
        else -> seniorFee()
    }
}

さらに単純化できました。

if - then - else 構文で書いたときに比べて、プログラミングがわかりやすくなったと思います。

他にもこのような書き方もできます。

fun fee() {
    // ユーザの情報から子供ならtrueを返し、子供料金を返す
    if (isChild()) return childFee()

    // 大人ならtrueを返し、大人料金を返す
    if (isAdult()) return adultFee()

    // 上記以外はシニア料金を返す
    return seniorFee()
}

上記の場合は、if同士が疎結合なため、子供料金を返す処理に何かしらの変更があったとしても他のif文に影響を与えません。

まとめ

今回、コードをリファクタリングする上で基本になる箇所を紹介しました。もちろん他にもいろいろな方法があります。重要なのはそれが「理解しやすいコード」でコーディングされているか、とういう点です。
それだけで、コードの改修や新機能を追加する際の学習コストなどが改善されていきます。

私もまだまだ勉強中ですが、さらに「より良いコード」を目指して勉強をしていこうと思います。

PHONE APPLIについて

phoneappli.net
phoneappli.net