【C#】よりよい共通化の方法が知りたい

###前提・実現したいこと
以下のコードを改善したいです。
長いコードなので、経緯と意図を説明し、最後に問題のソースコードを載せます。

まず、メッセージ列挙体を受け取ってメッセージを返すGetMessageメソッドを考えました。

C#
1class MessageProvider
2    {
3        public string GetMessage(MessageKind messageKind)
4        {
5            switch (messageKind)
6            {
7                case MessageKind.TaskDone:
8                    return "all tasks are done";
9                case MessageKind.CommandLineArgumentsError:
10                    return "need some command-line arguments";
11                //...
12            }
13        }
14    }
15
16    enum MessageKind
17    {
18        TaskDone,
19        CommandLineArgumentsError
20    }

その後、メッセージだけではなく、メッセージ列挙体からメッセージのタイプ(エラー/通知)と
メッセージのIDも取得したいと考えました。
そこで、新たにGetMessageDataメソッドを考え、その内部で
getMessage,getId,getMessageLevelメソッドを呼ぶようにしました。

C#
1class MessageProvider
2    {
3        public const int FirstInfoIndex = 10001;
4        public const int FirstErrorIndex = 20001;
5
6        public MessageData GetMessageData(MessageKind messageKind)
7        {
8            return new MessageData
9            {
10                Id = getId(messageKind),
11                Message = getMessage(messageKind),
12                MessageLevel = getMessageLevel(messageKind)
13            };
14        }
15
16        private string getMessage(MessageKind messageKind)
17        {
18            //...
19        }
20
21        private int getId(Enum messageKind)
22        {
23            return Convert.ToInt32(messageKind);
24        }
25
26        private MessageLevel getMessageLevel(Enum messageKind)
27        {
28            var id = getId(messageKind);
29            if (id < FirstInfoIndex)    
30                throw new ArgumentException("implimentation error");
31            return id < FirstErrorIndex ?
32                MessageLevel.Info :
33                MessageLevel.Error;
34        }
35    }
36
37    public class MessageData
38    {
39        public string Message { get; set; }
40        public MessageLevel MessageLevel { get; set; }
41        public int Id { get; set; }
42    }
43
44    publicenum MessageLevel
45    {
46        Info,
47        Error
48    }

ここで、メッセージはプロジェクト毎に存在するため、
メッセージ列挙体を受け取ってメッセージを返すgetMessageメソッドは
各プロジェクトに必要ですが、getMessageLevel,getId,それからGetMessageDataメソッドも
共通のアルゴリズムとして利用できることに気が付きました。
そこで以下のものを共通化して各プロジェクトから参照したいと思いました。
・MessageDataクラス
・MessageLevel列挙体
・GetMessageDataメソッド
・getIdメソッド
・getMessageLevelメソッド
また、以下のものを各プロジェクトで実装したいと思いました。
・FirstErrorIndex
・FirstInfoIndex
・getMessageメソッド
・MessageKind列挙体

上記を実現するために悩んだ挙句以下のようなコードを書きました。(全体なので非常に長いです)
Project1名前空間が多数のプロジェクトのうちの1つで、
LowLevelAssembly名前空間が共有プロジェクトです。

C#
1namespace Project1
2{
3    class Project1MessageProvider : LowLevelAssembly.IMessageProvider
4    {
5        public const int FirstInfoIndex = 10001;
6        public const int FirstErrorIndex = 20001;
7
8        private Project1MessageKind _messageKind;
9
10        public Project1MessageProvider(Project1MessageKind messageKind)
11        {
12            _messageKind = messageKind;
13        }
14
15        public Enum MessageKind
16        {
17            get { return _messageKind; }
18        }
19
20        int LowLevelAssembly.IMessageProvider.FirstInfoIndex { get { return FirstInfoIndex; } }
21        int LowLevelAssembly.IMessageProvider.FirstErrorIndex { get { return FirstErrorIndex; } }
22
23        public string GetMessage()
24        {
25            switch (_messageKind)
26            {
27                case Project1MessageKind.TaskDone:
28                    return "all tasks are done";
29                case Project1MessageKind.CommandLineArgumentsError:
30                    return "need some command-line arguments";
31                case Project1MessageKind.SpecialError:
32                    return "failed to backup log files";
33                default:
34                    throw new ArgumentException("undefined");
35            }
36        }
37    }
38
39    enum Project1MessageKind
40    {
41        TaskDone = Project1MessageProvider.FirstInfoIndex,
42        CommandLineArgumentsError = Project1MessageProvider.FirstErrorIndex,
43        SpecialError
44    }
45}
46
47namespace LowLevelAssembly
48{
49    public class MessageData
50    {
51        public string Message { get; set; }
52        public MessageLevel MessageLevel { get; set; }
53        public int Id { get; set; }
54    }
55
56    public enum MessageLevel
57    {
58        Info,
59        Error
60    }
61
62    public class MessageDataCreator
63    {
64        private IMessageProvider _messageProvider;
65
66        public MessageDataCreator(IMessageProvider messageProvider)
67        {
68            _messageProvider = messageProvider;
69        }
70
71        public MessageData GetMessageData()
72        {
73            var messsageKind = _messageProvider.MessageKind;
74            return new LowLevelAssembly.MessageData
75            {
76                Id = getId(_messageProvider.MessageKind),
77                Message = _messageProvider.GetMessage(),
78                MessageLevel = getMessageLevel(messsageKind)
79            };
80        }
81
82        private MessageLevel getMessageLevel(Enum messageKind)
83        {
84            var id = getId(messageKind);
85            if (id < _messageProvider.FirstInfoIndex)
86                throw new ArgumentException("implimentation error");
87            return id < _messageProvider.FirstErrorIndex ?
88                LowLevelAssembly.MessageLevel.Info :
89                LowLevelAssembly.MessageLevel.Error;
90        }
91
92        private int getId(Enum messageKind)
93        {
94            return Convert.ToInt32(messageKind);
95        }
96    }
97
98    public interface IMessageProvider
99    {
100        int FirstInfoIndex { get; }
101
102        int FirstErrorIndex { get; }
103
104        string GetMessage();
105
106        Enum MessageKind { get; }
107    }
108}

このコードをどのように改善したらいいか、ご意見を下さい。
よろしくお願い致します。

##備考
以下は現在の実装に対するクライアントコードのサンプルとなります。

C#
1namespace Project1
2{
3    class ClientClass
4    {
5        public void ClientCode()
6        {
7            var provider = new Project1MessageProvider(Project1MessageKind.TaskDone);
8            var creator = new LowLevelAssembly.MessageDataCreator(provider);
9            var messageData = creator.GetMessageData();
10            Console.WriteLine($"ID:{messageData.Id } Level:{messageData.MessageLevel} Content:{messageData.Message}");
11        }
12    }
13}

##※追記事項
コード全体を改善することが第一の目標なのですが、
以下のようなクライアントコードによってメッセージデータが取得できるようになれば、
更に良いだろうと思っています。
以下は一例です。

C#
1namespace Project1
2{
3    class ClientClass
4    {
5        public void ClientCode()
6        {
7            var provider = new Project1MessageProvider();
8            var creator = new LowLevelAssembly.MessageDataCreator(provider);
9            var messageData = creator.GetMessageData(Project1MessageKind.TaskDone);
10            Console.WriteLine($"ID:{messageData.Id } Level:{messageData.MessageLevel} Content:{messageData.Message}");
11        }
12    }
13}

Project1MessageProviderクラスのインスタンスひとつにつきメッセージをひとつしか
取得できないような仕組みになってしまっているのは、決して意図したことではなく、
これ以外に実現方法が分からなかったからです。
本当はGetMessageメソッドの引数にはProjectMessageKind型の変数をとりたいですし、
その他のプロジェクトにおいても、そのプロジェクトに固有のメッセージ列挙体を引数としたいです。

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

回答1件

ベストアンサー

こんにちは。
コーディングがドツボにはまってる感じがしたので、深読み回答してみます。

まず要件を整理してみましたが、

メッセージからId、テキスト、種類を取得したい
プロジェクト毎に特有のメッセージを定義したい
クライアントは共通のコードでメッセージを読み取れるようにしたい

の3つで間違いはないでしょうか？

この場合、Messageに拡張性に乏しいenumを採用している理由が推測できなかったので、思い切って一から組み直してみました。

完全に元コードぶち壊しですが、以下のコードで目的は達成できるでしょうか？

csharp
1// ================================================================
2// low-level 共通アセンブリ内
3
4// メッセージの種別定義
5
6public enum MessageLevel
7{
8    Info,
9    Error
10}
11
12// メッセージ本体の抽象構造定義
13
14public abstract class Message
15{
16    public abstract int Id { get; }
17
18    public abstract string Text { get; }
19
20    public abstract MessageLevel Level { get; }
21}
22
23public abstract class Info : Message
24{
25    public override MessageLevel Level => MessageLevel.Info;
26}
27
28public abstract class Error : Message
29{
30    public override MessageLevel Level => MessageLevel.Error;
31}
32
33
34// 共通メッセージ定義
35
36public class TaskDone : Info
37{
38    public override int Id => 1;
39
40    public override string Text => "all tasks are done";
41
42}
43
44public class CommandLineArgumentsError : Error
45{
46    public override int Id => 2;
47
48    public override string Text => "need some command-line arguments";
49}
50
51public class SpecialError : Error
52{
53    public override int Id => 3;
54
55    public override string Text => "failed to backup log files";
56}
57
58
59// ================================================================
60// Project1 アセンブリ内
61
62// 追加メッセージ
63
64public class Project1HogeError : Error
65{
66    public override int Id => 20001;
67
68    public override string Text => "project1 local error";
69}
70
71
72// ================================================================
73// クライアントコード
74
75public void Main()
76{
77    var message = new TaskDone();
78
79    WriteLine($"ID:{ message.Id }, Level:{ message.Level }, Content:{ message.Text }");
80}

各メッセージをそれぞれMessage抽象クラスの実体として定義しています。この方法なら、Project固有のメッセージを後から自由に追加可能になります。各プロジェクト内でInfo Errorクラスを継承して定義します。
メッセージの内容や種別などは、メッセージ自体に持たせるのが普通だと思うので、Providerなどは使用せず、素直な実装にしています。
Idの使用用途がわからなかったので、各メッセージに固定で割り振る方法で定義しています。同様に、FirstErrorIndex FirstInfoIndexも使用用途がわからず、こちらは勝手に廃止しています。
上にも書いてますが、C#のenumは特に拡張性がない構文なので、このような場面では採用するべきではないです。適当なのは、メッセージ種別を分類するためのMessageLevelに使用するくらいでしょうか。
interfaceは特に定義していませんが、必要であれば適当に実装してください。

元の質問内容を思いっきり書き換えているので、本来の要件を満たせていない可能性もありますが、ご容赦ください。

投稿2016/09/05 02:18

tamoto

総合スコア4228

syogakusya

2016/09/05 04:06 編集

ご回答ありがとうございます。目から鱗でした。要件についてはおっしゃるとおりで間違いないです。 > enumは拡張性がない長年かかっていた霧が晴れました。考えてみれば、今まで列挙体の絡むコードで再三悩んできたように思います。 enumの使い方が誤っていたのですね。実はこのメッセージはアプリケーションのイベントログへの出力に使うもので、そのためメッセージのデータの一つとしてIDが必要となっています。 FirstErrorIndexなどは、IDからメッセージの種類が通知かエラーかを判別するために使っていました。 1-10000までが通知、10001-20000までがエラーといった仕組みなので、IDがわかれば種類もわかるということです。プロジェクトごとに通知とエラーのID範囲が違うことを想定し、このような実装にしました。追記: 申し訳ありません。少しだけ本来の意図とは違う部分もありました。メッセージについては、共通のメッセージなどは存在せず、各プロジェクトが使うメッセージは全てそのプロジェクトが提供するつもりで、共通したかったのは一部のアルゴリズムだけでした。しかし、メッセージについても共通メッセージを定義できるなら、尚のこと良いと思いました。ちょうど一部のメッセージを共通化したくなっていたところでした。しかし、どのようにしてIDを一意かつ連番にするかについてはまだ何も思いつきません。また、共通化されたメッセージのIDについても、プロジェクトごとにその共通メッセージに割り当てるIDを変える仕組みが必要になりそうです。宜しければ、もう少しお付き合い頂ければと思います。

tamoto

2016/09/05 04:32

コメント確認しました。「全てのメッセージIdを『一意』かつ『連番』に定義する」と、「プロジェクト側で追加のメッセージを定義したい」を同時に満たすのは"不可能"です。理由は、プロジェクト側で追加されたメッセージをベースアセンブリ側では区別できないからです。たとえIdを自動で連番に割り振る仕組みを構築したとしても、「同じメッセージ種別には同じIdが割り振られるようにする」方法がないのです。「『一意』なメッセージIdを定義する」となると、それはコーディング前の「仕様策定」の話で、その「仕様」を満たすようにするのなら、この回答のコードのように各メッセージクラスにIdをハードコードすればいいですし、それこそ「全てのメッセージをベースアセンブリの一つのenumにハードコードする」という最初の方法が最善手になり得ます。一つの提案ですが、IdというのがIdentify、その名の通り「一意にメッセージを類別できること」を目的としているのであれば、例えば"アセンブリ名+クラス名"を文字列として読み込ませたものをIdとするなど、整数型にこだわらないやり方を模索するのも手だと思います。この方法であれば、「一意性」と「拡張性」を両立することができます。

syogakusya

2016/09/05 05:09

コメントありがとうございます。なるほど、おっしゃるとおりだと思います。 IDは全体ではなく各プロジェクトで連番にしたいため、一つのenumではできそうにないです。もう少し考えてみます。ありがとうございました。

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