きれいなコードとは？(1)

プログラミングやってるときれいなコードを書きたいと考えることがある。でも『きれいなコード』の定義は人や組織、コミュニティによってさまざまです。

『きれいなコード』を目指す目的

目的でさえいろいろあると思います。

１．レビューしやすい
２．再利用しやすい
３．かっこよく見せたい

まぁこんなところでしょうか？

確かに、『汚いコード』というのは、読みにくいし、それをベースに作り変えるのは難しく、わけのわからぬまま再利用したら、作者にしかわからない隠れたバグに悩まされる可能性もある。ただ、『きれいなコード』が必ずしもわかりやすいコードであるわけでもないです。

短いコードは『きれいなコード』？

きれいなコードと言って最初に思いつくのは、短いコードではないでしょうか？同じ処理でも長くなったり短くなったりします。よくある例を挙げると、

/** 最大値 */
private static final int MAX = 150;
/** 最小値 */
private static final int MIN = 0;
/**
 * 入力値が範囲内であるかチェックする
 * @param input 入力値
 * @return true:範囲内の値, false:範囲外の値
 */
private boolean isCorrectValue(int input) {
    if(input <= MAX && input >= MIN) {
        return true;
    }
    else {
        return false;
    }
}

例えば上記のような範囲チェックのメソッドがあったとする（最近Javaをよく使っているのでJavaで書いてみました）
このメソッドは誰が見てもわかると思います。if文の中で範囲であるかチェックして条件に合ったらtrue、合わなければfalseを返しているだけですね。
これは、以下のように？演算子を用いて書き換えることができます(※定数宣言やコメントは省略しています)

private boolean isCorrectValue(int input) {
    return (input <= MAX && input >= MIN) ? true : false;
}

/** これでも行けるよね？という指摘がありましたので追記しました(2021.2.14 14:00) */
private boolean isCorrectValue(int input) {
    return input <= MAX && input >= MIN;
}

また、以下のようにも書き換えられます。

private boolean isCorrectValue(int input) {
    if(input > MAX) return false;
    if(input < MIN) return false;
    return true;
}

private boolean isCorrectValue(int input) {
    if(input > MAX) {
        return false;
    } else if(input < MIN) {
        return false;
    } else {
        return true;
    }
}

どれも同じ結果になるでしょう。
この中で一番短く書けているのは？演算子を用いて書いた2つ目のコード（2021.2.14 14:00追記のコードの方が短い）です。この演算子は理屈が分かれば難しくないが、使う場所によっては却ってわかりにくいコードとなることもしばしばです。
これくらいの内容の場合、どうやって書いてもさほど変わりませんが、個人的には3つ目の最大値と最小値をそれぞれでチェックして、条件に合わないと早々にreturnするコードが好きですね。単体テストでも、どの値を入力したときにreternされているのか確認しやすいですしね。

短く書くことよりも名前が大事

個人的にコーディングをするうえで一番大事にしているのは、名前付け（ネーミング）ですね。

なぜ、ネーミングが大事かと言えば、そのメソッドや変数が出てきただけで、何をしている処理かすぐにわかるからです。

例えば、以下のようなコードは意味が分かるでしょうか？

    if(check(a)) { 
        System.println.out("error!"); 
    }
    else {
        int b = calc(a);
        System.println.out(String.format("b:%02d", b)); 
    }

何となく、変数aをチェックして、エラーにならなかったら、計算して計算結果であるbを出力していることはわかりますが、何のチェックをしているのか、どんな計算をしているのかわかりませんよね^^;

例えば、変数aがString型（文字列型）で渡された値で数値に変換可能かチェックするのなら、isNumeric(String arg)というメソッドにすればいいし、入力範囲チェックならisWithinRange(int arg)などが考えられます。
calc()も足し算なのか掛け算なのか、もっと複雑な計算なのか、メソッドの実装を見に行かなければわかりませんね。

わかりやすいコードは、実行しているところを見るだけで、何の処理をしているのかわかるコードです。

処理の内容と名前の意味がずれてしまったら

実装していくうちに、最初は判定だけのメソッドだったが、表示の処理も追加してしまったり、判定の範囲（対象）が変わってしまったり、宣言したときの名称にそぐわない処理内容になってしまうこともあります。

その時の対処法は3つあります。

１．名前と合わない処理を外に出して、別のメソッドにする
２．名前を変えて、追加された機能も包含されていることを示す
３．気にせずそのままにする

まず１．は値の判定はisCorrect○○()で値の表示はshow○○()など別のメソッドに分けることです。そうすることで、名前と処理の不一致を避けられます。

続いて２．ですが、checkAndSet○○()などのメソッド名にし、値の判定と処理を包含しているメソッドであることを示します。敢えてメソッドを分けないのは、チェックと表示が必ず同じタイミングで行われることが分かっている場合には有効的だと思います。分けてしまうことで、呼び出し忘れのミスが発生するよりは、1つのメソッドで処理した方が、正確性は上がりますね。

そして３．ですが、これも２．とほぼ同じ思想です。処理が同時に行われることについて、共通認識があれば、特に問題ないでしょう。ただし、共通認識がない場合や別々に呼ばれた方が都合がよい場合などは、分けた方がよいでしょう。

グローバル変数は悪？

これもよく考察されるテーマだと思います。

以下は個人的な考え方ですが、どちらが最善ということはないと思います。

グローバル変数はどこで変えられるかわからないという問題がよく言われます。ただ、どこでも参照できるので簡単に実装できるというメリットもあります。変更と参照のタイミングを把握していれば、そこまで悪ではないと思います。呼ぶ頻度や変更される頻度が高い場合は、注意が必要です。思わぬタイミングで値の更新が行われたとき想定外の動きをする可能性があるからです。

この問題は突き詰めたら一種の『宗教』になるので、開発者や開発チームが共通認識でわかるように実装していれば問題ないでしょう。ただし、可能な限り、いつでもだれでもアクセスできる変数ではなく、クラスごとのメンバ変数にとどめスコープを限定するようにした方がよいでしょうね。

teratailに面白いスレッドがありましたので、紹介しておきます。
グローバル変数を一切使わない開発に参画したことがありますか？

おわりに

このテーマは本当に一種の『宗教』なので、チームの宗派に合わせるしかないでしょうね（笑）
ただ、共通認識として言えるのは、「今のチームメンバーが理解できることと数年後の自分や開発を引き継いだメンバーが理解できることが大事」ということです。
これは、プログラミングに限らず、どんな資料でも言えることです。

きれいに書くことで、
それを目にした人が見たくなる。
見たくなることで理解してもらえるスタート地点に立てる。
そして良い点や問題点を見つけてもらえる。
一目で見る気がなくなるものはレビューも中途半端になります。また、なんかわからんからと投げ出される可能性もあります。

きれいなコードの絶対的な定義はありませんが、第三者の立場で読みたくなるか否かを考えてみることはできますね。読むのが嫌になるコードはたぶんきれいなコードではないのでしょうね。

最後に、この話題に触れたので、有名な本を紹介しておきますね。

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)