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

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

新規登録して質問してみよう
ただいま回答率
85.50%
C#

C#はマルチパラダイムプログラミング言語の1つで、命令形・宣言型・関数型・ジェネリック型・コンポーネント指向・オブジェクティブ指向のプログラミング開発すべてに対応しています。

Q&A

解決済

4回答

7251閲覧

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

退会済みユーザー

退会済みユーザー

総合スコア0

C#

C#はマルチパラダイムプログラミング言語の1つで、命令形・宣言型・関数型・ジェネリック型・コンポーネント指向・オブジェクティブ指向のプログラミング開発すべてに対応しています。

1グッド

0クリップ

投稿2019/12/26 01:29

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

C#

1class TestConstructor 2 { 3 public string methodName { get; } 4 public string imageName { get; } 5 public double imageSerchThreshold { get; } 6 public int X_Adjust { get; } 7 public int Y_Adjust { get; } 8 public int threadSleepTime { get; } 9 public AdbComannd.Scene scene; 10 public double sceneVerificationThreshold { get; } 11 public int sceneVerficationMaxCount { get; } 12 13 public TestConstructor(string methodName,string imageName,double imageSerchThreshold, int X_Adjust,int Y_Adjust ,int threadSleepTime,AdbComannd.Scene scene,double sceneVerificationThreshold,int sceneVerficationMaxCount) 14 { 15 this.methodName = methodName; 16 this.imageName = imageName; 17 this.imageSerchThreshold = imageSerchThreshold; 18 this.X_Adjust = X_Adjust; 19 this.Y_Adjust = Y_Adjust; 20 this.threadSleepTime = threadSleepTime; 21 this.scene = scene; 22 this.sceneVerificationThreshold = sceneVerificationThreshold; 23 this.sceneVerficationMaxCount = sceneVerficationMaxCount; 24 } 25 } 26void TestMethod() 27 { 28 TestConstructor testConstructor = new TestConstructor("A_Serch", "A.png", 0.6, 0, 0, 3000,AdbComannd.Scene.Warning,0.6,3); 29 TestConstructor testConstructor2 = new TestConstructor("B_Serch", "B.png", 0.6, 0, 0, 3000, AdbComannd.Scene.Warning, 0.6, 3); 30 }
退会済みユーザー👍を押しています

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

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

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

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

m.ts10806

2019/12/26 01:39

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

退会済みユーザー

2019/12/26 01:58

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

2019/12/26 02:03

コンストラクタの定義が長いと言っているのか、インスタンス化の記述が長いと言っているのか、どちらですか?
退会済みユーザー

退会済みユーザー

2019/12/26 02:09

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

2019/12/26 02:12

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

2019/12/26 02:16

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

退会済みユーザー

2019/12/26 02:17

「"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

2019/12/26 02:22

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

2019/12/26 02:31

「渡す情報は変わらないけど、渡す情報を減らしたい」 ではないんですよね? 「渡す情報は変わらないけど、コンストラクタの記述を短くしたい」 ですか? コンストラクタは引数無しで、各プロパティにSetをつけるのもだめなんですよね? (これでも記述は長くなりますよね?) クラスに渡す情報量が変わらず、情報による違いを共通化出来ないなら、コーディング量は減らないと思うのですがどうでしょうか。 コーディング量は増えても、コンストラクタの記述が短くなれば良い、というわけでもないでしょうし。
Zuishin

2019/12/26 02:37

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

2019/12/26 02:44

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

回答4

0

ベストアンサー

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

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

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

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

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

投稿2019/12/26 03:10

退会済みユーザー

退会済みユーザー

総合スコア0

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

退会済みユーザー

退会済みユーザー

2019/12/26 03:21

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

退会済みユーザー

2019/12/26 03:23

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

2019/12/26 04:07

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

2019/12/26 04:15

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

2019/12/26 04:21

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

2019/12/26 04:27

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

2019/12/26 04:40

なるほどです。 理解しました。 ありがとうございます。
退会済みユーザー

退会済みユーザー

2019/12/26 08:19

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

0

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

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

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

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

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

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

投稿2019/12/26 01:57

fana

総合スコア11632

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

退会済みユーザー

退会済みユーザー

2019/12/26 02:14

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

2019/12/26 03:30

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

0

Bulder パターンを使ってみるのはどうでしょうか(参考)?

投稿2019/12/26 01:49

maisumakun

総合スコア145121

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

退会済みユーザー

退会済みユーザー

2019/12/26 01:57

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

0

目的が分からないのですが、ただ単純に引数に多数の変数を並べたくないということなら、クラスにまとめてそのクラスのオブジェクトを引数にしてはいかがですか? 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); ・・・中略・・・ }

投稿2019/12/26 02:41

退会済みユーザー

退会済みユーザー

総合スコア0

バッドをするには、ログインかつ

こちらの条件を満たす必要があります。

あなたの回答

tips

太字

斜体

打ち消し線

見出し

引用テキストの挿入

コードの挿入

リンクの挿入

リストの挿入

番号リストの挿入

表の挿入

水平線の挿入

プレビュー

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

ただいまの回答率
85.50%

質問をまとめることで
思考を整理して素早く解決

テンプレート機能で
簡単に質問をまとめる

質問する

関連した質問