C#のクラス設計でアドバイスをいただきたいです

Question

C#でIf文、Switch文、foreach文、LINQを多用しているクラスがあるのですが、可能ならもう少しコンパクトにしてパフォーマンスを上げたいと考えています。

###前提・実現したいこと
まず、ウェブ上で20個のラジオボタンを設定して、それより大きく分けて4種類のオブジェクトに分けています。
それぞれのオブジェクトで概ね500行程度のロジックを組んでいるのですが、自分が見ても長すぎると考えています。
重複したロジックをまとめて、すっきりさせたいです。

###発生している問題
冗長的なロジックによる見栄えの悪さ

###該当のソースコード
実際のコードの二つのケースを簡略化して、掲載しています。
1000文字超えてしまったので、外部にアップロードしました。
お手数ですが、以下をご覧いただければと思います。
プログラムソース

###試したこと
同じような関数をまとめようと思いましたが、LINQでの記述が各パターンごとに少しづつ違うため、うまくまとめれませんでした。

###補足情報(言語/FW/ツール等のバージョンなど)
C#
ASP.NET MVC
VisualStudio2013

Answer 1

ご参考までに、、、
C# のコーディング規則
１：上記にもありますように、型が分かる場合はvarを使うとすっきりして可読性も上がると思います。

２：TARGET_AとTARGET_Bをセットしている部分は、IEnumerable<AAA>を返すメソッドを用意すれば、1行でそれぞれコーディングできるのではないでしょうか。

ex)
TARGET_A = GetDbData(ABCD_SET, "A").ToList();
TARGET_B = GetDbData(ABCD_SET, "B").FirstOrDefault();

他にも、AAAクラスの中は、KeyId, Id, No, Type, Nameのみで考えたいですね。

上記は部分的なところではありますが、何かの足掛かりにでもなれば幸いです。

Comments

[FitD]

回答ありがとうございます。
C#のコーディング規則は初めて見ました、参考にさせていただきます。
>1
過去に分岐中にvarで定義したらエラーに苦しめられたので、最初から型を調べて定義していました。全体的に見直したほうがよさそうです。

＞２
なるほど、ex)の内容を参考にさせていただきます。
他も明示的に定義しなくても、構成上問題なさそうですね。
ありがとうございました。

Answer 2

public string A_id { get; set; }
public string A_no { get; set; }
public string A_type { get; set; }
public string A_name { get; set; }
public string B_id { get; set; }
...

と
X_(属性名)
という記述が繰り返されますが、

id,no,type,nameを持つクラスを作り、
...と思ったら既にある(DB_LIST)のでそれを使って

public partial class AAA{ //なぜpartialなのか
    public string Key { get; set; }
    public DB_LISt A { get; set; }
    public DB_LISt B { get; set; }
    public DB_LISt C { get; set; }
    public DB_LISt D { get; set; }
}

というふうに書けばいいんじゃないかと思いました。

が、
その後を見ると、
Aだけセットされたタイプ
Cだけセットされたタイプ
A,Bがセットされたタイプ
...
など
いろいろな種類を1つの"AAA"型に無理やり突っ込んでいる感じがするので
データ構造を根本的に整理しなおしたほうがいいと思います。

---

オブジェクト指向だ手続き型だと気にしているようですが、
手続き型としても読みづらいです。

Comments

[FitD]

回答ありがとうございます。
2番目のclass AAAが理解力不足のせいか、どう使えばいいのかわからないです。
また、データ構造はDB側の問題ということでしょうか？

それだと編集はできませんが、AAA型の問題であれば、AAB、AACのようにたくさんの型を作ることは可能です（実際にそれも考えてはいました）。

[FitD]

いろいろやっているうちに、何とか理解は追いついてきました。
AAAは、KeyとDB_LISTをA、B、C、Dにわけたものですね。
となると、AABで、KeyとDB_LIST、A,Bみたいに作ればいいのかなと思いました。

オブジェクト指向を気にしているのは、当初は全く気にせず作ってしまい、後で見直したら自分でも可読性が悪いと思ったので、作り直そうと考えました。
大本のほうが、IOとフローチャートでの設計をもとにコーディングしているせいだと思います。クラス設計はちゃんとできていないので、今回アドバイスをいただければと思ってました。

Answer 3

情報の追加の依頼で書こうと思いましたが、オブジェクト指向プログラミングの話になるのでいったん回答で書きます。

ソースコードを一瞬見ました。重複したロジックがあるので、いくつか関数をまとめるとスッキリしたコードになると思います。ただ、オブジェクト指向となると、掲載されたソースコードでは問題があります。

というのも、オブジェクト指向では「Aであるaをbでcする」という書き方で、意味のあるクラス名・メソッド名・変数名をつけることで、データやロジックに名前を付けて管理するのがすごく大きな利点です。つまり、何のためのなんのコードだかわからないのでは、オブジェクト指向にやりようがありません。

また、パフォーマンスについてですが、一見して特別重くなる部分はなさそうですがどこが問題になっていますか？一休さんのトラ退治の話では、絵に描いトラは捕まえようがないという話ですが、遅くなっていないコードもまた速くしようがありません。これに関しては、実行できるコードが必要です。

Comments

[FitD]

回答ありがとうございます。
やはり関数をまとめてすっきりしたコードにはできそうですね、設計した内容をそのまま自分の知識でできるコードに置き換えたのでこうなってしまいました。

オブジェクト指向の前にデータ・ロジックの意味づけが先に重要ですよね。通常のように設計書があれば問題ないのでしょうが、今回はコメントを整理することで対応してみます。

パフォーマンスは、実際にテストサーバーでレスポンス時間を計ってみたところ、他の機能よりも出力に10倍以上時間がかかっていたためでした。
パフォーマンスについては一旦質問から省くことにします。

[iwamoto_takaaki]

画面があるなら、画面上の意味から名前とればいいです。画面上から名前が取れないなら、設計時に画面の意味づけをせずに設計している可能性があります。

パフォーマンスに関しては、実際のコードの方の問題かなと思います。見てみないとわかりませんが、Webシステムでのパフォーマンスが悪い場合は、SQLが重いか、無駄なIOが起こっているかのどちらかです。

[FitD]

画面上には意味があるものはあります。ただ、teratail用にシンプルにするために、今回は抽象的な変数名にしてしまいました。
パフォーマンスに関しては、if文＋LINQによるSQLServerへの問い合わせが複数回行われているからだと思います。もう少し考えてみます。

[iwamoto_takaaki]

クラスを設計する際には意味毎にクラスを定義し、クラス変数を使ってやることをメソッドとして定義します。そのため元の意味を隠せばどの変数をクラスとすべきか判断が付きません。関連が深い変数をまとめることでそれっぽくも出来ますが、その場合、手続き型の書き方よりわかり辛いこともありえます。

クラスを作る際には名前には十分な考慮が必要というわけです。

パフォーマンスについては、実際のソースを見ないとなんともいえませんが、同じSQLを何度も発行していないか注意して下さい。もしその場合は、データをためておくListを作成して、見つからなかったらDBに問い合わせるといった方式に切り替えることをおすすめします。

[FitD]

クラス設計がしっかりできないのは、おそらく私のコーディングが手続型の書き方になってしまっているせいもあるかと思います。

パフォーマンス改善に関しては、Listに保存すればいいというのは、おっしゃる通りです。問い合わせ回数を減らす方向で見直したほうがよさそうですね。

[FitD]

いくつかのDBへの問い合わせを、先に格納したコレクションを見るようにしましたら、パフォーマンスが改善できました。
今回の改善だけでなく、過去のプログラムへも対応できそうなので、BAとさせていただきます。
ありがとうございました。

Answer 4

C#は、あまり分かりませんが、ぱっと見て一番問題だと思ったのは、コメントがないことです。
それともこれは、テラテイル用に意図的なものですか。

あとこれは、設計より記述の問題ではないですかと思いますが。
なので、そういうのは、ツールで測って見るといいんじゃないかと思いました。

SourceMonitorというのがあるようですよ
メトリクス計測 SourceMonitor 紹介

Comments

[FitD]

回答ありがとうございます。
コメントに関しては、確かにこちらには何も書いていませんでした。
コメントを書いたものを改めてアップしてみます。

ツールのご紹介ありがとうございます、早速使ってみます。