C# コンストラクタの引数が長くなりすぎる

Question

コンストラクタの引数が長くなりすぎます、コンストラクタの引数を長くしない方法はありますか？
プロパティにsetを追加して外部クラスから代入するのも考えましたが、あまりよく思えません。

class TestConstructor
    {
        public string methodName { get; }
        public string imageName { get; }   
        public double imageSerchThreshold { get; }
        public int X_Adjust { get; }
        public int Y_Adjust { get; }
        public int threadSleepTime { get; }
        public AdbComannd.Scene scene;
        public double sceneVerificationThreshold { get; }
        public int sceneVerficationMaxCount { get; }

        public TestConstructor(string methodName,string imageName,double imageSerchThreshold, int X_Adjust,int Y_Adjust ,int threadSleepTime,AdbComannd.Scene scene,double sceneVerificationThreshold,int sceneVerficationMaxCount)
        {
            this.methodName = methodName;
            this.imageName = imageName;
            this.imageSerchThreshold = imageSerchThreshold;
            this.X_Adjust = X_Adjust;
            this.Y_Adjust = Y_Adjust;
            this.threadSleepTime = threadSleepTime;
            this.scene = scene;
            this.sceneVerificationThreshold = sceneVerificationThreshold;
            this.sceneVerficationMaxCount = sceneVerficationMaxCount;
        }
    }
void TestMethod()
        {
            TestConstructor testConstructor = new TestConstructor("A_Serch", "A.png", 0.6, 0, 0, 3000,AdbComannd.Scene.Warning,0.6,3);
            TestConstructor testConstructor2 = new TestConstructor("B_Serch", "B.png", 0.6, 0, 0, 3000, AdbComannd.Scene.Warning, 0.6, 3);
        }

Comments

[m.ts10806]

「関数の引数を減らす方法」でしたら言語関係なく調べれば幾らでもでてくるのでは？
得てしてそもそもの設計が悪いものです。

[退会済みユーザー]

どのあたりが悪い設計でしょうか？初心者ですので教えていただけると嬉しいです

[workaholist]

コンストラクタの定義が長いと言っているのか、インスタンス化の記述が長いと言っているのか、どちらですか？

[退会済みユーザー]

インスタンス化の記述です、普通のライブラリを使用しているときにはここまで引数は長く指定することはないので自分の書き方がおかしいのかなと思いました。

[workaholist]

「"A_Serch", "A.png", 0.6, 0, 0, 3000,AdbComannd.Scene.Warning,0.6,3」
が長いと言っているのですよね？
短くしたいのであればこの情報を減らせば良いのでは？

[m.ts10806]

というより「クラス設計きちんとしてないから引数が多くなって混乱する」というところですね。
もしくはもっと基本設計、詳細設計をきちんとやったほうがいいかもしれません。
「製造しながら考える」と設計があってないようなものになります。

[退会済みユーザー]

「"A_Serch", "A.png", 0.6, 0, 0, 3000,AdbComannd.Scene.Warning,0.6,3」
「"B_Serch", "A.png", 0.3, 30, 0, 3000,AdbComannd.Scene.Warning,0.5,5」
画像処理に使うしきい値や、上限値等の減らせない情報が多くオーバーロードできずに困っています

[m.ts10806]

配列に詰め込んだら送受ともに1つになるんでは。
受け側で必要な内容チェックする必要はありますけど、トータルとしては軽くなるかと

[workaholist]

「渡す情報は変わらないけど、渡す情報を減らしたい」
ではないんですよね？
「渡す情報は変わらないけど、コンストラクタの記述を短くしたい」
ですか？
コンストラクタは引数無しで、各プロパティにSetをつけるのもだめなんですよね？
（これでも記述は長くなりますよね？）

クラスに渡す情報量が変わらず、情報による違いを共通化出来ないなら、コーディング量は減らないと思うのですがどうでしょうか。
コーディング量は増えても、コンストラクタの記述が短くなれば良い、というわけでもないでしょうし。

[Zuishin]

少なくとも画像に関する情報とシーンに関する情報の二種類あります。二つに分けるべきクラスが一つにまとめられてしまっている気がします。

[m.ts10806]

結局のところ何を実現したくて組んだコードか提示されないことにはZuishinさんがコメントされているように「気がする」にとどまり、回答も「なんとなく」ふわっとしたものになるので、的確なアドバイスにはならないですよ。
自身で初心者と仰る人のコードだけで要件まで把握するのは他者には困難です。

Answer 1

この手の問題には色々考えがあります。

1つは思い切ってpublicにしてしまい、コンストラクタ外で設定させることです。
この方法は分かりやすい反面、カプセル化やimmutable性に問題があり、また、あるメソッドを呼ぶ際に必要な値がすべては揃っているかも保証できないため、安易に採用できず、使いどころを選ぶ必要があります。

もう1つは他の方もおっしゃっているBuilderパターンです。
ただし、結局メソッドチェーンで値を登録しまくる必要があるので、記述の長さは別に解決しません。

私のお勧めは「名前付き引数を使い、コンストラクタの引数が多くても別に良しとする」ことです。
そもそもコンストラクタに引数が多いためにBuilderパターンを使うのは、引数のどの値がなにを示しているのか分かりにくいからであり、C#では名前付き引数を使えばこの問題は発生しません。
また、デフォルト引数やコンストラクタのオーバーロードも使えるので、どの引数が設定必須かといったことにも融通が利きます。

ちょっと話は変わりますが、引数が多いときは「本当に全部の引数が必要なのか？」「クラスが複数の役割を持ってしまっていないか？」という設計の見直しも大事かと思います。

Comments

[退会済みユーザー]

回答ありがとうございます。
名前付き引数を使うことにしました。
変数名の命名の見直し、クラスの設計を見直してみます。

回答者の皆様ありがとうございました。

[退会済みユーザー]

私見が混じりますが、変数名やメソッド名が長いだとか引数が多いだとかは、それが本当に必要ならソースの可読性を下げるものではないと思います(ただし、用途が明らかなローカル変数などは除く)
寧ろ見た目すっきりさせるために無理に変数名を省略したり、元の設計思想に反して変に引数を弄ることこそが可読性を下げる行為だと思います。

ですから、設計上引数が多いのが正しいのであれば、堂々と名前付き引数を使って良いと思います。

[workaholist]

すいません、教えて下さい。
名前付き引数を使ったらインスタンス化の記述が今回の例は短くなるのですか？
もしかしてimageSerchThreshold=0.6とかを規定値にするとかですか？

[YAmaGNZ]

短くするのではなく
TestConstructor test = new TestConstructor(methodName: "name",imageName: "Name"～
という感じで可読性を上げようということではないでしょうか？

[workaholist]

いや、質問が「コンストラクタの引数が長くなりすぎる」だから、更に長くなりますよね。

[YAmaGNZ]

質問はそうですが、NotionalKettleさんは「私のお勧めは「名前付き引数を使い、コンストラクタの引数が多くても別に良しとする」ことです。」と仰ってます。
ただ、引数が多いと何番目の値が何を示すものか分かりにくくなり可読性が下がるので、名前付き引数で可読性を上げようという話だと読めました。

で、質問者さんはそれを選択したというふうに考えました。

[workaholist]

なるほどです。
理解しました。
ありがとうございます。

[退会済みユーザー]

説明いただきありがとうございます。
まさに仰るとおりです。

Answer 2

引数で与えているそれらの値が，オブジェクト構築時点で絶対に必要なものであって，且つ，適切な値を自動で決定できない性質のものであるならば，それらの情報全てをどうにかして解決する手段をコンストラクタ引数で受けるのは仕方ないと思いますが，

• 複数の引数がひとまとまりの設定のような情報であるならば，「設定値の塊」という型に纏めた方が扱いが便利になるかもしれません．

（コンストラクタの見た目の引数の個数も減る）

• 情報そのものではなく「情報をどこかから取得するための手段」を与えておく，という形の方が便利かもしれません．

（設定値はどこか別の場所で管理されていて，TestConstructor側としては必要な時にその値を用いて処理すれば良い場合）

TestMethod()の例では，引数のうち同じ値を与えられているものがいくつか見られますが，
もし何かしら「デフォルト値」として考えられる性質のものが存在するならば，引数にデフォルト値を指定しておくとか，あるいはコンストラクタをオーバーロードしたりすることで，コンストラクタを呼ぶ側の負担を減らすことはできるとは思います．

Comments

[退会済みユーザー]

回答ありがとうございます。
コンストラクタの引数はオブジェクト構成時点で絶対に必要です、画像処理のしきい値等が含まれています。
情報をどこかから取得するための手段とは具体的にどのような実装でしょうか？
Json等にアウトプットしておいて必要に応じて取得のような流れでしょうか？

[fana]

interface ISettingGetter{
double imageSerchThreshold { get; }
int X_Adjust { get; }
...
}
みたいな．このinterfaceの実装は単に直値を持っているだけなのかもしれないし，どこかを参照していてその先から値を引っ張ってくるのかもしれない．

Answer 3

Bulder パターンを使ってみるのはどうでしょうか（参考）？

Comments

[退会済みユーザー]

回答ありがとうございます。
Bulder パターンと言うものは初耳です、拝見します

Answer 4

目的が分からないのですが、ただ単純に引数に多数の変数を並べたくないということなら、クラスにまとめてそのクラスのオブジェクトを引数にしてはいかがですか？　ASP.NET MVC のモデルとアクションメソッドみたいな感じ。具体的に言うと以下のようにします。

public class Model
{
    public string methodName { get; set; }
    public string imageName { get; set; }   
    ・・・中略・・・
    public int sceneVerficationMaxCount { get; set; }
}

class TestConstructor
{
    private Model _model;

    public TestConstructor(Model model)
    {
        this._model = model;
    }
}

void TestMethod()
{
    Model model = new Model
    {
        methodName = "A_Serch", 
        imageName = "A.png",
        ・・・中略・・・
        sceneVerficationMaxCount = 3
    };

    TestConstructor testConstructor = new TestConstructor(model);
    ・・・中略・・・
}