データメンバが増えすぎたクラスのリファクタリングについて

Question

C++タグをつけさせてもらいましたが、プログラミング全体に関わる設計に関しての質問です。

例えば、Playerクラスを作成しようと思った時、攻撃力、防御力、HP、経験値、誕生日などなど……複雑な仕様を盛り込むことになりました。

この時、攻撃力、防御力、HPを一つのStatusクラスにまとめたりなどするして、Playerクラスのデータメンバを減らす工夫をすることになるでしょう。

ですが、Playerクラスは仕様上、沢山のデータを持つ必要があります。これが3,4クラス以内に纏められるのなら良いのですが、それ以上となると、メンバ初期化の為のコンストラクタの引数が、バカみたいに多くなってしまいます。

こういった時、皆さんならどのようにリファクタリングするでしょうか？是非、教えていただきたいです。

Answer 1

ですが、Playerクラスは仕様上、沢山のデータを持つ必要があります。これが3,4クラス以内に纏められるのなら良いのですが、それ以上となると、メンバ初期化の為のコンストラクタの引数が、バカみたいに多くなってしまいます。

N個メンバ変数があってコンストラクタの引数がN個であるとき、

// 長方形
class rect {
  int x0, y0; // 左上隅の座標
  int x1, y1; // 右上隅の座標
public:
  rect(int x0, int y0, int x1, int y1);
  ...
};

rect r(1,2,3,4); // 4個

これを纏めたところで

class point {
   int x,y;
public:
  point(int x, int y);
};

// 長方形
class rect {
  point topleft;     // 左上隅の座標
  point rightbottom; // 右上隅の座標
public:
  rect(point topleft, poiny rightbottom);
  ...
}
...
rect(point(1,2), point(3,4)); // やっぱり4個

初期化にはやっぱりN個の引数が必要でしょ?
何か問題ですか?

Answer 2

まずリファクタリングという観点であれば、クラス図などにした上で、具体的にアプローチし、範囲を限定して、特定のパターンで行うと思います。仮定の話では難しいと思います。

というのをふまえた上で、戦闘用のパラメータもあれば、生産用のパラメータ用もあれば、トレード用のパラメータもあるでしょう。目的ごとに可搬性のいいパラメータにしておけば後で受け渡しも楽でしょうから、そのように小分けにします。

構築時はCompositeなものであれば、メンバに入れて、プレイヤーオブジェクトが作成されるときにそのまま構築します。そうでなければ後からattach/setするような作りにします。プレイヤーやパラメータが汎化できるようなものであれば、Factoryパターンかそれ系の仕組みを持たせます。本当に種類が多ければDIまで作るかもしれません。

いずれにしても、パラメータの数についてはあまり気にしません。目的や用途に応じて、使いやすい構成にするだけです。パラメータの数が多くなった/クラスの種類が多くなったときは、またそれぞれ工夫していきますが、それらは原則ケースバイケースです。既存のデザインパターンで効果的な場合もあるし、そうでない場合もあります。

Answer 3

こんにちは。

Playerクラスは仕様上、沢山のデータを持つ必要があります。

であれば、そのままたくさんのデータ・メンバを持つのも有りと思います。無理してデータ・メンバを纏める必要はないでしょう。纒めたほうが好ましいメンバのみ纏めるべきかと。

コンストラクタが問題ということですが、その多数のデータ・メンバの初期化元のデータはどこから取ってくるのでしょうか？　一般にはハード・コーディングではなく何か設定ファイルで設定することの方が多いように感じます。（多数の初期値を調整するたびにビルドするのはあまり現実的ではないケースが少なくないですから。）
コンストラクタでは小数の重要な値のみをパラメータとし、他の値はその重要な値に応じて設定ファイルから読み出すのがスマートな設計のように思います。そのような設計ならデータ・メンバの数が種別（class）によって異なっても、使う側は大差ない記述ができますし。（ファクトリー・パターンを使って全く同じにすることもできますね。）

それらのデータ群をファイルやデータ・ベースへ保存・回復するような機能も必要になるのではないでしょうか？　その回復処理と初期化処理を共通化することも視野に入ると思います。

Answer 4

... メンバ初期化の為のコンストラクタの引数が、バカみたいに多くなってしまいます。 ...

１. default 引数を設定する。

２. builder パターンを検討する。
参考

• なんぞこれ。コンストラクタパラメータが異常に大杉る･･･。バカなの？死ぬの？そういう場合はBuilderパターンを検討してみよう。

http://zecl.hatenablog.com/entry/20091117/p1

• 生成に関するパターン Builder

https://fa11enprince.hatenablog.com/entry/2014/05/07/011000

Answer 5

他の方の回答とかぶっているところが多いですが、以下のような感じにすると思います。

• コンストラクタとは別に初期化メソッドを作成する。
• 初期化メソッドに渡す引数を最小限にする。
• メンバ変数の初期化処理もファクトリーパターンメソッドに分ける。
• 初期化時の値が決まっているパラメータは外部ファイルから読み込む。

例：

class Player
{
private:
    // 位置座標
    int posion_x;
    int posion_y;
    
    // ステータス
    Status status;

public:
    // コンストラクタ
    Player()
    {
        // ここで重い処理はしない
    }

    // 初期化メソッド（引数は最小限にする）
    void Initialize(int posion_x, int posion_y)
    {
        this->posion_x = posion_x;
        this->posion_y = posion_y;
        this->status = Factory::CreatePlayerStatus();
    }
};

Answer 6

リファクタリングじゃないので、回答になってないかも知れませんが、、、

初期化済みのオブジェクトを用意し、そのコピーを作成するようにする、、ではだめでしょうか?
ゲームだったら、戦士系とか、魔法系とか、、の主要なものを用意する。