質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

89.99%

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

解決済

回答 1

投稿 編集

  • 評価
  • クリップ 2
  • VIEW 1,358

canvas

score 60

質問

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

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

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

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

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

別質問で立てている「円クラスを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));

 追記

とりあえず、今回試した2つのルールを実践した結果、小さなクラスが結果的に出来上がりましたが、そのおかけで自然と「座標に関する振る舞いは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クラスが行ったりとなるのかなと思っています。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 1

checkベストアンサー

0

ちょっと書いてみました。
その本は読んでないので回答の質はあまり期待しないでください。(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のなにがしかの戻り値で依存することになりそうですけどね)。

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

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2016/08/14 20:26

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

    キャンセル

  • 2016/08/15 12:53

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

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 89.99%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる