コードレビュー「このコードっておかしいですか？」

Question

#質問

書籍「ThoughtWorksアンソロジー」の「第5章 オブジェクト指向エクササイズ」

上記書籍自体はまだ未購入で読んでおりませんが、ネット上でこの本で紹介されている「オブジェクト指向エクササイズ」について触れているページを幾つか見て気になっていたので実践してみました。

実践してみたのは良いのですが、なんだか良くわからなくなってしまいました。
「よくわからなくなった」という部分が質問箇所でして、いかに掲載するコードのコメントに記載しておりますので、そちらのコメントを参照頂けると幸いです。

以下のコードでは「オブジェクト指向エクササイズ」の以下の２つのルールを実践したつもりです。

ルール: インスタンス変数２つ
ルール:プリミティブ禁止

別質問で立てている「円クラスをMVC的に実装したいです！」のコードをリファクタリングする目的で練習がてらに取り組んでみました。

コード

コード内のコメントで「Q:」が先頭についているコメント箇所が自分が特におかしくないか気になっている箇所です。
アドバイス頂けると大変ありがたいです！

class Circle {
	
	private point: Point;
	private radius: Length;
	
	constructor(point: Point, radius: Length) {
		this.point = point;
		this.radius = radius;
	}
	
	//左上を起点としたxy座標を返す
	get position() {
		return this.point.cordinates;
	}
	
	//円の中心を起点としたxy座標を返す
	get centerPosition() {
		var positionAtLeftTop = this.point.cordinates;
		
		//Q.ここは新しいPointクラスのインスタンスを生成して返すべきですか???
		return {x: positionAtLeftTop.x.value + this.radius.value, y: positionAtLeftTop.y.value + this.radius.value};
	}
	
}


class Point {
	
	private x: Cordinate;
	private y: Cordinate;
	
	constructor(x: Cordinate, y: Cordinate) {
		this.x = x;
		this.y = y;
	}
	
	get cordinates() {
		return {x: this.x, y: this.y};
	}
	
	
	
}

class Cordinate {
	
	private cordinate: number;
	constructor(value: number) {
		this.cordinate = value;
	}
	
	get value() {
		return this.cordinate;
	}
	
}



class Length {
	
	private length: number;
	constructor(value: number) {
		this.length = value;
	}
	
	get value() {
		return this.length;
	}
	
}

//Q.依存性の注入を愚直にすると、引数がこんなことになってしまうのですが大丈夫でしょうか？
var circle = new Circle(new Point(new Cordinate(10), new Cordinate(10)), new Length(10));

追記

とりあえず、今回試した２つのルールを実践した結果、小さなクラスが結果的に出来上がりましたが、そのおかけで自然と「座標に関する振る舞いはCordinateクラスでしよう」とか、そういった発想が生まれやすいなという実感はありました。

flied_onionさんへの返答

LengthをCordinateから生成できない気がするんですが、Lengthは2つのCordinateで求められるべきじゃないかな、と。
Cordinateが「二次元の原点座標からの距離」と考えるならLengthもCordinateでいいような気もしてます。

円の半径をLength型で表していますが、Lengthを2点の座標から表すことにすると円の半径をどう表したら良いのでしょう。
円の中心点と外周を結ぶ線が半径ですから、円の半径をLength型で定義するためには円の中心点がまずxy座標ではどこで、そして、外周上の点はx, y座標のどこにするかを決めないと半径を出せないと思うので、なんだか小難しいですね。。。
そうすべきものなのでしょうか？

Lineクラスみたいなものであれば、初期化時に始点、終点を決めて生成するというのは納得です。

Q.ここは新しいPointクラスのインスタンスを生成して返すべきですか???

逆に聞きますが、Pointでないならそれは何を返してるんですか？
それ(設計)次第かと。

メソッドの戻り値を何にして呼び出し元に渡すかは確かにケースバイケースですよね。
単純にxy座標だけ知りたければ、{x: 10, y: 10}みたいな戻り値で十分であるということですよね。

少なくともpositionとcenterPositionは同じ物を返した方が一貫性があっていいと思います。

これは本当ですね！ ここは統一すべきだと感じました。ありがとうございます。

私にはこれが依存性の注入に見えませんでした(元がプリミティブだからかな)。
前はCircle内のPointをコンストラクタに渡されたx,yでコンストラクタの中でnewしてたのを、
Pointを受け取るようにして解消したってことでしょうか？

その通りです！ 別質問で詳しく投稿しています。
https://teratail.com/questions/44223

今はCordinateクラスもLengthクラスもPointクラスも特にメソッドらしいメソッドがないですが、長さの単位変換をLengthクラスが行ったり、座標変換をPointクラスが行ったりとなるのかなと思っています。

Answer 1

ちょっと書いてみました。
その本は読んでないので回答の質はあまり期待しないでください。(Code CompleteとUncle Bobの本は読んでます)
そういうエクササイズと思って眺めてちょっと気になった点を書いてみます。

LengthをCordinateから生成できない気がするんですが、Lengthは2つのCordinateで求められるべきじゃないかな、と。
Cordinateが「二次元の原点座標からの距離」と考えるならLengthもCordinateでいいような気もしてます。
→ 訂正：「一次元の原点座標からの距離」の間違いでした。

---

Qに対して。回答というよりは感想に近いです。

Q.ここは新しいPointクラスのインスタンスを生成して返すべきですか???

逆に聞きますが、Pointでないならそれは何を返してるんですか？
それ(設計)次第かと。

Pointでない少し違った意味を持つPositionなるものを用意するなら、違う事もあるでしょう。
ワールド座標・ローカル座標・変換などコンテキストを勝手に解決してくれる機能有するなら変わってくるかなとも妄想しましたが、それでもやっぱり変わらないかなぁ…Pointが今のところ単純な情報しか返さなそうなので。
Pointでない場合、Circleの中心からまたCircleを作りたいとき、その値をそのまま使ってPointになるのか、使い手は不安になるかもしれませんね。
少なくともpositionとcenterPositionは同じ物を返した方が一貫性があっていいと思います。

Q.依存性の注入を愚直にすると

私にはこれが依存性の注入に見えませんでした(元がプリミティブだからかな)。
前はCircle内のPointをコンストラクタに渡されたx,yでコンストラクタの中でnewしてたのを、
Pointを受け取るようにして解消したってことでしょうか？

---

追記分に関して回答します。

円の半径をLength型で表していますが、Lengthを2点の座標から表すことにすると円の半径をどう表したら良いのでしょう。

すいません、「二次元の原点座標からの距離」と書いてしまいましたが、「一次元」の間違いです。
つまりLengthは直線距離と考えています。
「2点間」という情報を持たせるならベクトルになるでしょう。
で、Cordinateも距離と捉えられ、( x:Cordinate, y:Cordinate はどちらも原点からの直線距離 )

たとえば原点(0,0) 半径 5 の円Aの半径の長さ(Length)と、点(5,0) が中心点の円Bの x座標(Cordinate)
があったとして、どちらも x軸上の原点からの距離と見えます。つまりどちらも同じ物に見えます。

点(0,5)が中心点の円Cがあって、円Aと円Bの中心点の距離(5:Length)と 円Aと円Cの中心点の距離(5:Length)に差を設ける(前述のベクトルの様に)なら良いんですけど、そうでないなら意味のある区分けになっていない気がしました。
図形から離れて、金銭を扱うプログラムでMoneyクラスを設けたとして、今の貯金額(Cordinateに相当)とこれから預金する額(Lengthに相当)を分けて考えている様に見えて不自然だなと感じただけです。
この辺りも設計次第なのでしょうし、強くそうした方が良いというわけでもないです。

単純にxy座標だけ知りたければ、{x: 10, y: 10}みたいな戻り値で十分であるということですよね。

むしろ x,y座標なら、コンストラクタで円の中心をPointで指定させておいて何事かとは感じそうな気はします。座標はPointという契約ではなかったのかと。(Circleのコンストラクタがその形式も受け入れるならそれでもいいです）
toRawCordinates とか、外部ライブラリとの連携のために、リテラル型の情報に返してくれるといったメソッドなら、そういう戻り値でもいいと思います。

その通りです！ 別質問で詳しく投稿しています。

そちらは見てませんがDIというと型の交換を可能にしたいなと思うので（個人的にです）、
先の例で言えばLengthが高機能になったり、Vectorが生まれても、
Cordinate, Length, Vector いずれもがMeasurableを実装していてCircleの初期化半径としてはMeasurableを渡せばいいみたいにしたくなりますね。いや、かなり適当に書いてますが。
それによって、CircleはLength, Vectorに依存しなくなるという(現実的にはCircleのなにがしかの戻り値で依存することになりそうですけどね）。

ざっくばらんに書いてみました、これ以上は深く考えるつもりはないです。
駄文失礼しました。

Comments

[canvas]

ご回答ありがとうございます！コメント欄ですとタグが機能しないため、質問文にてレスさせて頂きました！

[canvas]

追記ありがとうございます。頂いた回答を参考に再考してみたいと思います。