クラスの設計に関して以下１，２のどちらがより良いでしょうか。

2020/10/27 03:42

C言語での開発経験はありますが長いブランクがあります。オブジェクト指向はC言語の構造体とそれを設定・編集・取得する関数群からスタートしていると認識しています。ただし、オブジェクト指向言語による開発経験は全くありませんので、私が知らないことは多々あるのだろうと認識しています。逆に言えば、今回投稿したのも自分の知識不足が原因で判断つきかねているので質問した次第です。今後のためにも有益なご意見や提言ならぜひともよろしくお願いします。

Zuishin

2020/10/27 04:37 編集

- そもそもその二つのメリットとデメリットの差が小さすぎてどうでもいいです。 - バージョンによる差異を吸収するクラスを作るべきです。 - 例外は握りつぶすべきではありません。 - ネストが深すぎます。 - CompareTo が雑です。 - 他にもありそう。オブジェクト指向で作り直すなら、逐次翻訳するのではなく設計からやり直すのが良いと思います。それをしないのなら作り直す意味もさほどありません。

2020/10/27 05:23

Zuishinさん、ご指摘ありがとうございます。 1.大差なし、という感じなのですね。他者様のご指摘も考慮して検討します。 2.この部分は検討材料とさせていただきます。 3.誤解を招く表記（コード）で申し訳ありませんでした。組み込む際は上位でCatchしてユーザー向けのメッセージを表示してシステム終了となります。 4.ガード節、早期リターンを採用してネストを浅くします。 5.バージョンの比較ではハマるケースがあると聞いています。その辺りも考慮して調査・検討・改修します。 6.コードから全体が想像できたのかもしれませんね。コードレビューをありがとうございます。現場には私一人しかおらず、コードレビューも受けられる環境ではないので非常にありがたいご指摘です。追記：7.作り直す意味もさほどありません。→それもそうだと思いますが、オブジェクト指向に則った新規クラスやメソッドの作成により、改修コードが端的になり可読性も増していると実感しています。僅かながらではありますが新しい技術を取り入れつつ内製ソフトをリファクタリングしたいと考えています。

2020/10/28 02:38 編集

失礼しました。以前、４年ほど前に開発現場で仕様書に「プログラミングにおいては個人のカラーを出すことは極力避けてください」旨を目にしました。今、出向先で保守・改修している手続き型で作成された内製ソフトにおいて新しい技術＝オブジェクト指向を取り入れながらリファクタリングしたいと考えています。人員は私一人です。しかし、私はオブジェクト指向での開発経験がないため、自己研鑽の意味も込めてネット検索や書籍などの知識をベースに作業にあたっています。また、過去に推奨された手法が現在は非推奨となっているなど時流の変化もあります。そのため、現状の開発現場における定石などがどうなっているのか知ることで、自己流にならないように気を付けたいと思っています。それがこちらのサイトの意図に反してしまったのだと思います。上記を理解して下されば幸いですが、ご迷惑もおかけしたくないのでこれにてクローズします。いただいた意見や回答を糧にさせていただこうと思います。ありがとうございました。

退会済みユーザー

2020/10/28 02:40 編集

あくまで問題解決のためのサイトなので、そういう設計手法関係の勉強は自分で調べたり、実際に作ったりしてみながら場数踏むしかないんじゃないですかね。 githubの有名どころのライブラリのソースを眺めてみるのもよいでしょう。下の記事とかは、言語関係なく良い事が書いてあるので一通り読むべきだと思います。 https://qiita.com/alt_yamamoto/items/25eda376e6b947208996

2020/10/28 02:40

ご指摘、ありがとうございます。

行動規範の内容に同意します

回答2件

ベストアンサー

まず、VersionのListは利用する側が把握する必要があるのでしょうか？
提示されている例だけですと、最新バージョンが取得できればいいだけでリストを公開する理由が分かりません。
なので、内部で閉じてしまっていいように見えます。

またこの機能だけですとわざわざインスタンスを作成して利用するのではなくSharedで作ってもいいように見えます。

VBNET
1Public Class SampleApp3
2
3    Private Shared versionlist As List(Of Version) = Nothing
4
5    Public Shared Function GetInstalledLastestVersion() As Version
6        If versionlist Is Nothing Then
7            CreateVersionList()
8        End If
9        'このように毎回作成しても問題ないかも
10        'CreateVersionList()
11        'というか、リストを作成する過程で最新バージョンが分かるのでは？
12
13        '最新バージョンを求めて返す
14
15    End Function
16
17    Private Shared Sub CreateVersionList()
18        versionlist = New List(Of Version)
19
20        'バージョンのリストを作成
21    End Sub
22End Class
23

利用側

VBNET
1Dim App3Version = SampleApp3.GetInstalledLastestVersion()

最新バージョンを取得するだけの機能であれば、yuki23さんの仰るようにクラスにする必要はなく関数でいいかと思います。

VBNET
1Module Sample
2    Public Function GetInstalledLastestVersion(appname as String) As Version
3        'この中でリストを作って最新バージョンを返す
4    End Function
5End Module

投稿2020/10/27 03:45

総合スコア10489

2020/10/27 03:59

>まず、VersionのListは利用する側が把握する必要があるのでしょうか？説明が不足しておりました。はい、把握する必要があります。 VersionのListを取得した後、Formに表示してどのバージョン（のDLL）を使うかユーザーに選択してもらいます。（内製ソフトは数値計算アプリで、バージョンの違いによりDLLからの戻り値が微妙に異なり、その辺りも比較・検討の材料にしています）そこで、呼び出し側が把握する必要があるVersionのListは呼び出し側orクラス側のどちらが保持するのが妥当なのか知りたいと思っています。

2020/10/27 04:34

１番目のサンプルですが、プロパティとして用意するのであれば ByRef versions As List(Of Version) といった感じで引数で外部から与えられるという形にするのはおかしいく感じられます。というか１番目、２番目共通ですが、このクラスが作り出す情報を引数で貰うという設計になっているメソッドが謎です。２番目のようにプロパティをなくすとしても Public Function GetVersionList() As List(Of Version) Public Function GetLastestVersion() As Version といったメソッドのほうが一般的かと思います。

2020/10/27 05:55

YAmaGNZさん、返信ありがとうございます。 >引数で外部から与えられるという形にするのはおかしいく感じられます。元々は案1、かつコンストラクターで・メソッドCreateInstalledSampleApp1Versions を呼び出していました。そうすればクラスをNewしただけで・プロパティーPublic Property Versions As List(Of Version) にアクセス（=バージョンを取得）できます。ただし、それだと呼び出し側ではListを作成した自意識が生まれず、逆に迷惑かと思いました。（←これは私が時折おこなう手法なのですが、これも良くはないのでしょうか）そこで、次に各メソッドに引数を何も渡さない形に修正しました。（メンバー変数やプロパティーにクラスのメソッドが自由にアクセスできるので当たり前の形にしました）すると、・プロパティーPublic Property Versions As List(Of Version) をクラスで保持する意味があるのだろうかと疑問を持ちました。（クラスのメソッドでListを作成してリターンすればよいだけの話であり、クラスで保持する必要はないのではないかと思いました）そこで、明示的に引数で渡すことを思いつきました。これが違和感の元だと思います。 >このクラスが作り出す情報を引数で貰うという設計になっているメソッドが謎です。私のアイデアは一般的に違和感があることが分かっただけでも助かりました。ご指摘の通り、メンバー変数やプロパティーを引数で渡す形はやめようと思います。ちなみに、・プロパティーPublic Property Versions As List(Of Version) は呼び出し側、もしくはクラス側のどちらが宣言・保持するのが適切だと思われますか。 Zuishinさんがおっしゃる通り、どちらでも良い軽度な問題でしょうか。

2020/10/27 06:27

そのあたりはどちらでもいいと思いますが、プロパティで実装するのであれば Public ReadOnly Property Versions As List(Of Version) とReadOnlyとすべきかと思いますし、クラス側に実装すべきです。呼び出す側にプロパティを実装するということは、クラスに対して呼び出し側のインスタンスを通知するという形になり今回の場合意味があるとは思えません。このクラスがどのような位置づけなのか分かりませんが、利用側が「Listを作成した自意識」を持つ必要があるのでしょうか？今回の例ですと、Listを作成した自意識は必要なく、Getすれば（プロパティだろうがメソッドだろうが）値が得られれば問題ないように思えます。

2020/10/27 07:16

「バージョン違いで複数インストールされたアプリのバージョン情報のList」という意味のあるデータとして、またそのデータを操るメソッドを一つにまとめたクラスとして設計するのが適切だと思うに至りました。かつ、YAmaGNZさんのご指摘を受けてReadOnlyにしようと思います。ご助言に感謝いたします。もう一つだけご教授ねがえませんでしょうか。先ほども触れたように、コンストラクター内でListを生成する手法はどう思われますか？（←Newしただけである特定のメンバー変数やプロパティーに情報が設定され、呼び出し側では即座にそれらを参照できる）やめた方がよろしいですか。一般的ではないでしょうか。呼び出し側で・Dim appInfo As New SampleApp1() してクラスのインスタンスを生成した後に・appInfo.CreateInstalledSampleApp1Versions() でListを作成すべきでしょうか。私としましては、コンストラクターで設定できるメンバー変数やプロパティーならば、呼び出し側でNewした直後に参照できていいのではないかと考えていますが、いかがでしょうか。アドバイスのほど、よろしくお願いします。

2020/10/27 08:19

＞コンストラクターで設定できるメンバー変数やプロパティーならば、呼び出し側でNewした直後に参照できていいのではないかと考えていますがこの意味が分かりませんが、コンストラクタで初期化したのであれば、Newした直後にアクセスできることになります。呼び出し側でappInfo.CreateInstalledSampleApp1Versions()とListを作成する意味があるのですか？コンストラクタでListを生成せずにCreateInstalledSampleApp1Versionsメソッドをコールすることに意味があるのであればそのようにすべきですし、そうでないのであればわざわざメソッドをコールする必要はないでしょう。結局のところは貴方がどのように設計するか次第です。

2020/10/28 01:32

YAmaGNZさん、おっしゃる通り設計者次第なのだと思います。オブジェクト指向では、利用したいクラスのインスタンスをNewし、そのインスタンのメソッドを呼び出して処理をおこなってもらう。また、コンストラクターはメンバー変数の初期化などの用途につかわれます。それを踏まえると >呼び出し側でappInfo.CreateInstalledSampleApp1Versions()とListを作成する意味があるのですか？意味がある→そうするのが流れかと思います。ただし、バージョン情報のListを作成する際には、呼び出し側からの何らの情報も必要としないため、呼び出される側のコンストラクター内でもメンバー変数の初期化と称して作成が可能です。バージョン情報のListを作るためのアクションは誰が起こすべきなのか悩み、質問した次第です。呼び出し側が「メソッド呼び出し＝作ってください」といってから作られるのか、呼び出される側がNewされた時点（＝コンストラクター内）で「メンバー変数の初期化＝作っておきます」なのか、です。 >結局のところは貴方がどのように設計するか次第です。とおっしゃられている通り、それがYAmaGNZさんの回答なのかもしれませんが、なにか指針のようなものや多くの人が取る手段、もしくは開発現場でのセオリーなどを御存じでしたら教えていただけないでしょうか。重箱の隅をつつくような話ではございますが、よろしくお願いいたします。

退会済みユーザー

2020/10/28 01:52

https://teratail.com/cautions_question ここの[問題・課題が含まれていない質問]を読んでください

2020/10/28 01:58

ですから、newとCreateInstalledSampleApp1Versionsを分離する意味があるのかということです。 newのタイミングと同じタイミングでListを作成するのがまずいのであれば分離すべきですし、そうでないのであれば趣味の域になるのではないでしょうか。ただ、メソッドとして実装するのであればCreateInstalledSampleApp1Versionsと作成するメソッドを作るのではなくGetVersionsと取得メソッドを用意してその中で作成されていなければ作成してListを返すという実装のほうが私は好みです。例えばこのクラスをnewしただけでデフォルトとして最新バージョンで動作するといった仕様であればコンストラクタでリストを作成し最新バージョンを取得するといった実装が必要になります。例のように実装すべき仕様によってどうすべきかは変わりますし、現状提示されている情報だとお好きにしてくださいといった話しかできない部分かと思います。

2020/10/28 02:35

YAmaGNZさん、度々のご回答ありがとうございます。そう、仕様が抜け落ちていました。話がかみ合わないはずですね、ご迷惑おかけしました。その点を踏まえて再考します。ご意見を承れたことに感謝しています。先ほど、質問内容が本サイトの意図に反しているとのご指摘もあったので、これにてクローズします。重ね重ね、お礼申し上げます。ありがとうございました。

行動規範の内容に同意します

クラスにする必要がないので、メソッド1つにするのが良いと思います

追記
GetInstalledSampleApp1/2LatestVersion メソッドは、SampleApp1/2 クラスではなく List(Of Version) クラスに対する操作なので、SampleApp1/2 に所属するのは不適切です。
そもそも List に対する Max メソッドで書けると思いますので、わざわざメソッドを実装する必要がありません。

となると App1/2 クラスに残るのは CreateInstalledSampleApp1/2Versions メソッドだけになりますが、1個しかメソッドがないクラスをわざわざ作る必要があるのか、という話になります。

投稿2020/10/27 03:13

編集2020/10/27 12:20

総合スコア1448

2020/10/27 07:17

yuki23さん、ご回答ありがとうございます。説明不足でご迷惑をおかけし、誠に申し訳ありませんでした。 Listを保持する意味がなく、最新バージョンのみの取得ならおっしゃる通りだと思います。

2020/10/27 12:21

回答追記しました

2020/10/27 12:40

List クラスも Version クラスも十分よくできた完成されたクラスであり、それをただラップしただけのクラスを作るのは有害無益だと思います。

2020/10/28 00:13

yuki23さん、追記をありがとうございます。確かにおっしゃる通りだと思います。貴重なご意見をありがとうございます。ぜひとも今後の参考にさせていただこうと思います。ただ、以下「（メソッドは一部抜粋）」と記しましたようにその他のメソッドも定義されています。誤解を招く表記をしてしまい申し訳ありませんでした。 >利用するためにバージョン情報に関するクラスを以下のように設計・作成しました。（メソッドは一部抜粋です）使用しているサードパーティー製のアプリのバージョン把握・利用に関する今回の改修範囲は、その把握方法と表示&ユーザー選択部分となり、それを受けた計算部分には及びません。ちなみに、計算部分ではVersionクラスは利用せずにずオリジナルな方法をとっているため、データを変換するメソッドなども必要になり、それらを含めてクラス化しようと考えた次第です。