11

現在、9 つの異なるコンストラクターを持つクラスを変更しています。全体として、このクラスの設計は非常に不十分だと思います...クラスが非常に多くのコンストラクタを持つのは設計が悪いのではないかと思います。

最近、クラス (以下のコードの SomeManager) をリファクタリングして再設計しようとして、このクラスに 2 つのコンストラクターを追加したため、問題が発生しました。これにより、単体テストが可能になり、すべてのメソッドが静的であることに依存しなくなります。ただし、他のコンストラクターは、クラスの先頭から約 100 行下に隠れていたため、コンストラクターを追加したときにそれらを見つけることができませんでした。

現在起こっていることは、これらの他のコンストラクターを呼び出すコードは、以前は静的であったため、既にインスタンス化されている SomeManager クラスに依存しているということです....結果は null 参照例外です。

だから私の質問は、どうすればこの問題を解決できますか? コンストラクターの数を減らそうとすることによって?既存のすべてのコンストラクターに ISomeManager パラメーターを持たせることによって?

確かに、クラスに 9 つのコンストラクターは必要ありません。...さらに、このファイルには 6000 行のコードがあります。

上記で話しているコンストラクターの検閲された表現を次に示します。

public MyManager()
    : this(new SomeManager()){} //this one I added

public MyManager(ISomeManager someManager) //this one I added
{
    this.someManager = someManager;
}

public MyManager(int id)
    : this(GetSomeClass(id)) {}

public MyManager(SomeClass someClass)
    : this(someClass, DateTime.Now){}

public MyManager(SomeClass someClass, DateTime someDate)
{
    if (someClass != null)
       myHelper = new MyHelper(someOtherClass, someDate, "some param");
}

public MyManager(SomeOtherClass someOtherClass)
    : this(someOtherClass, DateTime.Now){}

public MyManager(SomeOtherClass someOtherClass, DateTime someDate)
{
    myHelper = new MyHelper(someOtherClass, someDate, "some param");
}

public MyManager(YetAnotherClass yetAnotherClass)
    : this(yetAnotherClass, DateTime.Now){}

public MyManager(YetAnotherClass yetAnotherClass, DateTime someDate)
{
    myHelper = new MyHelper(yetAnotherClass, someDate, "some param");
}

アップデート:

皆様の回答に感謝します...彼らは素晴らしいものでした!

私がやったことの最新情報を提供したいと思っただけです。

null 参照例外の問題に対処するために、ISomeManager を受け取るように追加のコンストラクターを変更しました。

現時点では、この特定のクラスのリファクタリングが許可されるかどうかについて、私の手は結ばれているため、時間があるときに再設計するクラスの todo リストにフラグを立てます。現時点では、SomeManager クラスをリファクタリングできたことをうれしく思います...この MyManager クラスと同じくらい巨大で恐ろしいものでした。

MyManager を再設計するときは、機能を 2 つまたは 3 つの異なるクラスに抽出する方法を探します... または、SRP を確実に遵守するために必要な数のクラスにします。

最終的に、特定のクラスに最大数のコンストラクターがあるという結論には達していませんが、この特定のインスタンスでは、それぞれ 2 つまたは 3 つのコンストラクターを持つ 2 つまたは 3 つのクラスを作成できると思います。

4

16 に答える 16

21

クラスは 1 つのことだけを行う必要があります。非常に多くのコンストラクターがある場合は、あまりにも多くのことを行っていることを示す兆候のようです。

複数のコンストラクターを使用して、さまざまな状況でオブジェクトのインスタンスを正しく作成することを強制しますが、9 は多くのように思えます。そこにはインターフェイスがあり、ドラッグできるインターフェイスの実装がいくつかあると思います。それらのそれぞれには、それぞれの専門分野に関連する 1 つからいくつかのコンストラクターがある可能性があります。

于 2009-01-29T06:35:27.263 に答える
16

できるだけ少なく、
必要なだけ。

于 2009-01-29T07:20:20.230 に答える
7

クラスに 9 つのコンストラクターと 6000 行があるのは、コードのにおいがする兆候です。そのクラスをリファクタリングする必要があります。クラスに多くの責任がある場合は、それらを分離する必要があります。責任が類似しているがほとんど逸脱していない場合は、継承の実装を検討して、インターフェイスと異なる実装を作成する必要があります。

于 2009-01-29T06:48:08.250 に答える
3

クラス内のコンストラクターの数を任意に制限すると、膨大な数の引数を持つコンストラクターになる可能性があります。毎日、100個の引数を持つコンストラクターよりも100個のコンストラクターを持つクラスを取ります。コンストラクターが多数ある場合、それらのほとんどを無視することを選択できますが、メソッドの引数を無視することはできません。

クラス内のコンストラクターのセットは、M 個のセット (各セットは単一のコンストラクターの引数リスト) を指定されたクラスの N 個のインスタンスにマッピングする数学関数と考えてください。ここで、クラスはそのコンストラクターの 1 つでBara を取ることができ、次に示すように、クラスはコンストラクター引数として a を取るとします。FooFooBaz

    Foo --> Bar
    Baz --> Foo

次のような別のコンストラクターを Bar に追加するオプションがあります。

    Foo --> Bar
    Baz --> Bar
    Baz --> Foo

これは Bar クラスのユーザーにとっては便利ですが、Baz から Bar への (Foo を介した) パスが既にあるため、追加のコンストラクターは必要ありません。したがって、これが判断のコールが存在する場所です。

しかし、 という名前の新しいクラスを突然追加し、そこからQuxのインスタンスを作成する必要があることに気付いた場合はBar、コンストラクターをどこかに追加する必要があります。したがって、次のいずれかになります。

    Foo --> Bar
    Baz --> Bar
    Qux --> Bar
    Baz --> Foo

また:

    Foo --> Bar
    Baz --> Bar
    Baz --> Foo
    Qux --> Foo

後者の方がクラス間でコンストラクターがより均等に分散されますが、それがより良い解決策であるかどうかは、それらが使用される方法に大きく依存します。

于 2009-01-29T07:16:32.047 に答える
3

答え: 1 (注射剤に関して)。

このトピックに関する素晴らしい記事は次のとおりです。依存性注入のアンチパターン: 複数のコンストラクター

要約すると、クラスのコンストラクターは依存関係を注入するためのものであり、クラスはその依存関係についてオープンである必要があります。依存関係は、クラスが必要とするものです。それが欲しいものでも、それが欲しいものでもありませんが、なくてもできるものです。それは必要なものです。

したがって、オプションのコンストラクター パラメーター、またはオーバーロードされたコンストラクターを使用することは、私には意味がありません。唯一のパブリック コンストラクターは、クラスの依存関係のセットを定義する必要があります。それはあなたのクラスが提供している契約であり、「あなたが私にIDigitalCamera、 、ISomethingWorthPhotographingおよびIBananaForScaleを与えるなら、私はあなたが想像できる最高のくそったれをあなたに与えますIPhotographWithScale。しかし、あなたがそれらのいずれかを軽視するなら、あなたはあなた自身です」と言っています.

以下は、Mark Seemann による記事で、正規のコンストラクターを使用するより細かい理由のいくつかを説明しています: State Your Dependency Intent

于 2014-02-06T19:32:05.357 に答える
2

リファクタリングについて心配しなければならないのは、このクラスだけではありません。他のすべてのクラスも同様です。そして、これはおそらく、コード ベースである絡み合ったかせの中の 1 つのスレッドにすぎません。あなたは私の同情を持っています... 私は同じ船に乗っています。上司はすべてのユニット テストを望んでいますが、ユニット テストができるようにコードを書き直すことは望んでいません。それを機能させるために、いくつかの醜いハックを行うことになります。静的クラスを使用しているものをすべて書き直して使用しないようにする必要があり、おそらくもっと多く渡す必要があります...または、シングルトンにアクセスする静的プロキシでラップできます。そうすれば、少なくともシングルトンをモックして、そのようにテストできます。

于 2009-01-29T06:37:41.377 に答える
2

あなたの問題は、コンストラクターの数ではありません。9 つのコンストラクターを持つことは通常より多いですが、必ずしも間違っているとは思いません。それは確かにあなたの問題の原因ではありません。本当の問題は、初期設計がすべて静的メソッドだったことです。これは、クラスが密結合しすぎている特殊なケースです。現在失敗しているクラスは、関数が静的であるという考えに縛られています。問題のクラスからそれについてできることはあまりありません。このクラスを非静的にしたい場合は、他の人がコードに書き込んだ結合をすべて元に戻す必要があります。クラスを非静的に変更してから、すべての呼び出し元を更新して、最初にクラスをインスタンス化します (またはシングルトンからクラスを取得します)。すべての呼び出し元を見つける 1 つの方法は、関数を非公開にして、コンパイラーに通知させることです。

6000 行では、クラスはまとまりがありません。頑張りすぎているのかもしれません。完璧な世界では、クラス (およびそれを呼び出すクラス) をいくつかの小さなクラスにリファクタリングします。

于 2009-01-29T06:51:31.567 に答える
1

そのタスクを実行するには十分ですが、クラスは単一の責任のみを持つべきであると述べている単一責任の原則を思い出してください。それを念頭に置いて、9 つのコンストラクターを持つことが理にかなっているケースはおそらく非常に少ないでしょう。

于 2009-01-29T06:41:41.027 に答える
1

クラスに実際のコンストラクターを 1 つだけ持つように制限します。実際のコンストラクターは、本体を持つコンストラクターとして定義します。次に、パラメーターに応じて実際のコンストラクターに委任する他のコンストラクターがあります。基本的に、コンストラクターをチェーンしています。

クラスを見ると、本体を持つ 4 つのコンストラクターがあります。

public MyManager(ISomeManager someManager) //this one I added
{
    this.someManager = someManager;
}

public MyManager(SomeClass someClass, DateTime someDate)
{
    if (someClass != null)
       myHelper = new MyHelper(someOtherClass, someDate, "some param");
}

public MyManager(SomeOtherClass someOtherClass, DateTime someDate)
{
    myHelper = new MyHelper(someOtherClass, someDate, "some param");
}

public MyManager(YetAnotherClass yetAnotherClass, DateTime someDate)
{
    myHelper = new MyHelper(yetAnotherClass, someDate, "some param");
}

最初のものはあなたが追加したものです。2 つ目は前の 2 つと似ていますが、条件があります。最後の 2 つのコンストラクターは、パラメーターの型を除いて非常に似ています。

3 番目のコンストラクターを 4 番目のコンストラクターにデリゲートするか、その逆にするかして、実際のコンストラクターを 1 つだけ作成する方法を見つけようとします。最初のコンストラクターは、古いコンストラクターとはまったく異なることをしているため、適合できるかどうかはよくわかりません。

このアプローチに興味がある場合は、Refactoring to Patterns の本を探してから、Chain Constructorsページにアクセスしてください。

于 2009-01-29T08:26:31.017 に答える
0

オーサーは: NONE

言語ディランを見てください。それは他のシステムを持っています。

コンストラクターのインスタンスで、他の言語でスロット (メンバー) にさらに値を追加します。「init-keyword」を追加できます。次に、インスタンスを作成すると、スロットを必要な値に設定できます。

もちろん、'required-init-keyword:' を設定することができ、使用できるオプションは他にもあります。

それは機能し、簡単です。古いシステムが恋しいです。コンストラクタ (およびデストラクタ) の記述。

(ところで、それでも非常に高速な言語です)


于 2010-03-25T14:18:22.157 に答える
0

コンストラクターは、そのクラスのインスタンスを作成するために必須の引数のみを持つ必要があります。他のすべてのインスタンス変数には、対応する getter および setter メソッドが必要です。これにより、将来新しいインスタンス変数を追加する予定がある場合に、コードが柔軟になります。

実際、オブジェクト指向の原則に従っています -

  1. 各クラスの設計は、低結合と高結束を目指します
  2. クラスは、拡張に対してはオープンである必要がありますが、変更に対してはクローズされている必要があります。

    次のようなデザインが必要です-

    import static org.apache.commons.lang3.Validate.*; public class Employee { プライベート文字列名; プライベート従業員() {}

    public String getName() {名前を返します。}

    public static class EmployeeBuilder { private final Employee 従業員;

    public EmployeeBuilder()
    {
        employee = new Employee();
    }
    
    public EmployeeBuilder setName(String name)
    {
        employee.name = name;
        return this;
    }
    
    public Employee build()
    {
        validateFields();
        return employee;
    }
    
    private void validateFields()
    {
        notNull(employee.name, "Employee Name cannot be Empty");
    }
    

    } }

于 2015-02-22T16:33:17.507 に答える
0

確かに、クラスには、クラスに必要な数のコンストラクターが必要です...これは、悪い設計が引き継がれるという意味ではありません。

クラス設計は、コンストラクターが終了後に有効なオブジェクトを作成する必要があります。1 パラメータまたは 10 パラメータでそれを実行できる場合は、それで問題ありません。

于 2009-01-29T06:40:43.243 に答える
0

このクラスは、かなり多くのことを行うために使用されているように私には思えます。クラスをリファクタリングして、より特化したいくつかのクラスに分割する必要があると思います。次に、これらすべてのコンストラクターを取り除き、よりクリーンで、より柔軟で、より保守しやすく、より読みやすいコードを作成できます。

これはあなたの質問に対する直接的な回答ではありませんでしたが、クラスに 3 ~ 4 個を超えるコンストラクターが必要な場合は、おそらくいくつかのクラスにリファクタリングする必要があるという兆候だと思います。

よろしく。

于 2009-01-29T06:48:56.953 に答える
0

私があなたのコードから見ることができる唯一の「正当な」ケースは、それらの半分がコードから削除しようとしている廃止された型を使用している場合です。私がこのように作業しているときは、コンストラクターのセットが 2 つあることが多く、そのうちの半分は @Deprecated または @Obsolete とマークされています。しかし、あなたのコードはその段階をはるかに超えているようです....

于 2009-01-29T06:50:18.637 に答える
0

複数のコンストラクターを持つクラスには、複数の責任があると思います。しかし、その反対について納得していただけるとうれしいです。

于 2012-12-19T15:11:13.940 に答える
0

私は通常、いくつかのデフォルトパラメータを持つものを持っています。コンストラクターはオブジェクトの最小限のセットアップのみを行うため、オブジェクトが作成されるまで有効です。さらに必要な場合は、静的ファクトリ メソッドを作成します。このような種類:

class Example {
public:
  static FromName(String newname) { 
    Example* result = new Example();
    result.name_ = newname;
    return result;
  }
  static NewStarter() { return new Example(); }

private:
  Example();
}

これは実際にはあまり良い例ではありません。より良い例を考えて編集できるかどうかを確認します。

于 2009-01-29T06:57:04.800 に答える