見通しのいいコードを書くために | 東北工業大学ソフトウェア技術研究会

見通しのいいコードを書くために

📅2024-12-16

見通しのいいコードを書くために

皆さん、こんにちは。K.Y.(HN Rerurate_514)です。本日は「綺麗なコードを書くをために」と題しまして、見通しの良いコードを書くために必要なことを解説します。以下、読まなくてもいい導入。

私はよく友達や先輩にコードのレビューとまではいきませんが、他人のコードを見る機会が多いです。これは第一弾の記事でも話したことなのですが、まあコードが大体汚いです。これは驕りでもなんでもなく、平気でグローバル変数を使ったり、私が書いた単一責任のクラスに謎のコード（元のコードと全然関係ないコード）を挿入されたりと、その枚挙に遑がありません。そのおかげか、私は楽しいリファクタライフと解析ライフを送っています。(やりたくない) まあこれは冗談なのですが、とにかく人が見てもわかりやすいコードを書くべきです。ただ、どうしても綺麗なコードを書くのが難しい言語も存在します。例えば、JavaScriptなんかは特にそうです。柔軟性が高い言語ほど、綺麗かそうでないかが顕著に表れます。この言語の場合はさらに綺麗に書いていたとしてもどうしても限界があるという不思議な言語です。

授業ではまったくもって綺麗なコードを書くための方法を教えませんから、今回で解説します。

追いやすいコードを作成する。

追いやすいコードというのは基本的にきれいなコードです。スパゲッティではないというのは、とても素晴らしいことです。しかし、プログラミングではそのスパゲッティを簡単に作る材料がそこら中に転がっています。私たちは頭を使ってそのスパゲッティの材料でスパゲッティを作らないようにしなければならないのです。

大域変数は悪

皆さん、自分で書いたコードでバグが発生した際には、その動作を目で見てトレースするかと思います。その時に、変数があっち行ったりこっち行ったりしては、いつまでたってもそのトレースが終わらないですよね。

自分も同じような状況に遭遇したことがあります。例えば、こんな状況です。

function printInfo(){
	console.log("あなたは", id, "番で名前は", name, "です。");
}

このコードのみ書かれたprintInfo.jsファイルがあるとします。このファイルだけを見てみると、idとname変数が何なのかわかりません。おっと、これはどういうことなのでしょうか。

これを調査してみると全く別のファイルindex.htmlにて定義を発見しました。

<script>
	var id = 231
	var name = "tarou"
</script>

これは動作してみるとおそらくあなたは231番で名前はtarouです。と出力されるはずです。

これを実行してみると、

あなたは6435番で名前はhanakoです。

と出力されました。これは事前に期待していた動作とは違っていますね。またこれを時間をかけて確認してみると、

id = 6435
name = "hanako"

と書いてあるのを見つけました。

...

正直、こんな無駄なことはしてられません。あっちこっち行ったり来たりしていては時間がいくらあっても足りません。これはグローバル変数が引き起こした悪行の一つです。確かにグローバル変数は特にアルゴリズムを考える必要がなく、頭空っぽにしてコーディングできます。しかし、そのコードはたいていスパゲッティであり、もう少し頭を使ってコーディングする必要があります。先ほどのコードですら、

function printInfo(id, name){
	console.log("あなたは", id, "番で名前は", name, "です。");
}

引数を使用するだけでいいんです。

なぜ、わざわざグローバル変数を使ってまで見通しの悪いコードを書くのでしょうか？それとこの前、こんなコードにも会いました。

class Sample{
	let id = 0
	async printInfo(nameArg){
		let name = nameArg;
		let age = 0;
		let skills = [];
		let fetchedData;
		id++;
		
		await fetchedDataFromDB()
		
		function dainyu(){
			name += "さん"
			age = fetchedData.name
			skills = fetchedData.name

			console.log(全部出力)
		}
		
		async function fetchedDataFromDB(){
			fetchedData = await なんかふぇっちする(id);

			dainyu()
		}
	}
}

どうやら関数で切り分けしてるかと思いきや、関数の中で関数を呼び出しています。さらにnameやageといった変数があちこちで呼び出されています。これでは形だけローカルのグローバル変数になってしまっています。これは私が実際に体験したもので、実際にはもっと長く複雑なコードでした。

グローバル変数はかなり密結合な設計を引き起こします。大抵、値の操作は引数のみで十分です。(最も状態管理などになると話は変わってきます。) しかし、ただのアルゴリズムにグローバル変数の出る幕はありません。なるべく、値の受け渡しは引数やコンストラクタなどに留めましょう。

インスタンス変数は不変限定!!!

実はミュータブルなインスタンス変数もかなりグローバルであるといえます。皆さんの中には、スコープがクラスの中で限定されているのだから問題ないだろう、と思っている人もいるかもしれません。カプセル化すればいいだろう、とも考えているかもしれません。しかし、値を変更するメソッドがどこで呼び出されるかわからない関係上、インスタンス変数はグローバルであるといえます。

例えば、授業でこんな感じのコードが、カプセル化についての授業で出ました。(一部改変)

class MyClass(object):
	def __init__(self):
		self.__value1 = 0

	def setValue(self, value):
		self.__value1 = value
	
	def printValue(self):
		print(self.__value1)

カプセル化まではいいと思います。しかし、考えてみてください。このsetValueメソッドがいつ、どこで呼び出されるかわかりません。ただ、いつどこで呼びされるかわからない問題はしょうがないです。問題は、そのメソッドで変更が行われる__value1についてです。結局、これがいつ変更されるかわかりません。

インスタンス変数というのは本質的に不変(イミュータブル)であるべきであり、その値はコンストラクタによって初期化されるべきです。あまり可変(ミュータブル)なインスタンス変数の乱用は避けてください。これもかなり密な設計です。

私が先輩の書いたコードをリファクタしようとしたとき、このインスタンス変数が様々なメソッドに相互作用して簡単にメソッドを切り出せないという問題に直面したことがあります。

インスタンス変数を使用するときは不変なものにしましょう。ただ、KotlinとかDartとかだといいのですが、Pythonで不変な変数を作ろうとするとちょっと工夫が必要になります。今回は解説しません。

ifとかelseとかネストしちゃだめだからね？

これはまあ大体のWebサイトとか記事とか本に書いてあることなのですが、ifやswitchとかwhenとかの条件分岐は正直かなり見通しの悪いコードを生み出しやすいです。

特にifが大量にあるコードを誰が読みたいでしょうか？

if(a == 1){
	if(isShown){
		if(b != 1){
			if(!isSelected){
				print("hello, World");	
			}
		}
	}
}

こんなコードを書かれたらひとたまりもありません

基本的にifの中にifを追加してはいけません。上記のコードだって、

if(a != 1) return
if(!isShown) return
if(b == 1) return
if(isSelected) return
print("hello, World");

といったように書き換えることができます。このifの中にifを入れずに早期にreturnすることで見通しの良いコードになります。条件分岐は頭の中で条件となっている変数のことやelseの中身、そこで変更されている変数のことなど、考えなければいけないことが沢山あります。この早期returnをなるべく活用するようにしましょう。

余談ですが、これは私が高校生の時に書いたコードです。

fun alignRealmObject(realmObject: RealmObject) {
        val max = getMaxRealmData(1, realmObject) + 1
        for (i in 0 until max) {
            val obj = getObj(i,realmObject)
            var objFun = getObj(i,realmObject)

            if (obj == null) {
                for (j in 0 until max) {
                    objFun = realm.where(realmObject::class.java)
                        .equalTo(Id, i + j)
                        .findFirst()

                    if (objFun != null) break
                }
                dataBack(i, objFun, realmObject)
            }

            if(obj is MusicInfo && obj.title == "") {
                realm.executeTransaction {
                    obj.deleteFromRealm()
                }
            }

            Log.d("realmAligning", "realm -> aligning... $i times")
        }
        Log.d("realmAlignSuc", "realm -> align >> Success")
    }

このようにifにifを重ねたり、forも併用すると更にカオスになります。

最後に

正直な話、初心者はこういった綺麗なコードを書いたとて、すぐには意味がないかもしれません。私の友達はまず、エラーの内容を読まない、「エラーがでた」「ここわからん」という一文のみで質問してくる、構文の意味が分かってないなど、それ以前の問題が結構ありました。

この記事は、エラーを事前に防ごうといったニュアンスの記事です。エラーが出てからの対処法はそれぞれの環境で違いますから、やはり心持ちが大事かと思います。