Zenn
😇

【実録】こんな実装はやめてほしい

2023/11/28に公開
#
考え方
#
実装
#
構成
tech

う〜ん、あまり良くない実装な気がする...

先日、あまり良くない構成をしている実装を見た。
簡単な模倣構成を踏まえ、改善策等を少し考えてみる。

クラス構成

以下のクラス構成を考える。

①独立したクラス(1~3)から、共通クラスのSetDBを呼び出す
↓
②共通クラスで参照クラスのCreateDataを実処理として呼び出す

クラス1〜3

・Do()
→ 引数なし
→ 共通クラスのSetDBを引数(data1)を渡して呼ぶ
→ 「クラス3」は「共通クラス」を継承し、共通クラスのプロパティ「data3」に値を設定する

共通クラス

プロパティ「data3：string」を持つ

・SetDB()
→ エントリポイント
→ 各クラスから引数(data1)を受け取る
→ data2を生成し、moldDataにdata1、data2を渡す
→ CreateDBにmoldDataを渡し、登録を行う

・MoldData(data1, data2)
→ moldData構造体(モデル）に整形し、返却する
→ attribute1=data1、attribute2=data2、attribute3=data3(プロパティ)とする

・CreateDB(moldData)
→ 参照クラスにおけるCreateDB(moldData)を呼ぶ
→ 登録の可否をboolで受け取る

参照クラス

・CreateDB(moldData)
→ DB登録処理を行う
→ 可否をboolで返却する

課題①

MoldDataの引数の存在意義が揺らぐ

→ 関数は機能や処理を独立して切り離す思想を持つべきであると考える

引数は関数とその他の処理のゲートウェイ的な役割を果たすべきで、
つまり、関数の内部の処理に影響を与える要因は可能な限り引数のみにしたい！

今回のように、内部の処理（=構造体の生成とプロパティ設定）に外部の変数が設定された場合、関数という単位で処理を区切る必要性が薄れてしまう。スコープが意味わからんことになる。

課題②

追加要件に弱すぎる

改修によりmoldDataに一律で新たに3つのプロパティを設定したい！という要件が通った場合、
・クラス1〜2はattribute3~5に、クラス3はattribute4~7に新規プロパティを設定する
・attribute3は呼び出し元のクラスによって異なる値を設定する
という処理が求められてしまう。

上記ケースでは異なる仕様がクラス3の1通りだが、複数ある場合は目も当てられない。
異なる仕様が2つあるだけでこの様である。

commonClass.go
// goにクラスないですが、雰囲気だけ伝わって...

/* 例) クラス4からの呼び出しは
	attribute1=data2
	attribute2=data4
	attribute3=data1　　を設定する */
	
package commonClass
// クラス3でのみ設定されるプロパティ
var Data3 string = ""

// クラス4からの呼び出しフラグ
var IsRouteClass4 bool = false

// data構造体
type data struct {
	attribute1 string
	attribute2 string
	attribute3 string
	attribute4 string
}

// データ整形
func moldData(data1 string, data2 string, data4 string) {
	var moldData data

	if !IsRouteClass4 {
		// クラス4経由の場合
		moldData.attribute1 = data2
		moldData.attribute2 = data4
	} else {
		// クラス1, 2, 3経由の場合
		moldData.attribute1 = data1
		moldData.attribute2 = data2
	}

	if Data3 != "" {
		// クラス3経由の場合
		moldData.attribute3 = Data3
		moldData.attribute4 = data4
	} else if !IsRouteClass4 {
		// クラス4経由の場合
		moldData.attribute3 = data1
	} else {
		// クラス1, 2経由の場合
		moldData.attribute3 = data4
	}
}
	
// こりゃひどい

改善案①

共通クラスでデータ整形をやめよう

個別のクラスで同じ構造体を利用するが、設定するプロパティ数が異なる
→個別クラスでデータ整形をしよう

個別のクラスで異なる使用を共通にするんじゃあない
自分を整形してから世界(他の処理)に出るんだ、自分磨きをしろ

改善案②

構造体の設計を見直そう

attributeとかいう便利な言葉で、何が入ってるかよくわからんし何を入れるかよくわからん仕様になっている

  • attribute1　→ ID
  • attribute2　→ Name
  • attribute3　→ CommodityName
  • attribute4　→ CommodityID

みたいな感じで、意図のわかる項目にしてしまえばこんなことにはならないはずだ...
反面、設定の自由度は減るが得られる利点と比較すれば軽微なものである

まとめ

簡単にではあるが、問題点と改善点を挙げた。
実際は、引数をそのまま次の処理の引数に渡していたりとあまり好ましくない処理が多い現実である。

誰が見てもわかる、意図の実装を汲み取れるコードの重要性を改めて実感した。

仕事なので、粛々と取り組むが、できるだけ負債を産まない実装を心がけたい。
自分が混沌を生む側にならないように...

Discussion