こんにちは。
なんとなく思ったことを回答してみます。
solid原則とかのガイドラインはきちんと学習しているわけではないので、的外れな意見かもしれません。
まず、インターフェースは「書かれた以上の仕事をしない」のがルールです。
例えば、IGetDataインターフェースですが、こんな感じの定義だと仮定してみます。
csharp
1interface IGetData<T>
2{
3 T GetData();
4}
これであれば、「GetDataメソッドはT型のデータを返す」という一文以上のことはしません。「データを返せない場合」があれば、それは「想定外の動作」となり、例外を送出する以外の選択肢は無いです。実装側の設計によっては、「データが無い場合はdefault(T)」というのも考えられますが、あまりいい設計とは思いませんね。
そして、IGetDataインターフェースが最初から「データを取得できない可能性もある」パターンをサポートする場合、それをインターフェース定義に盛り込みます。例えば以下のような感じでしょうか。
csharp
1interface IGetData<T>
2{
3 // C#7のValueTuple風記述です。Tupleクラスを使ったり、Resultクラスとかを定義しても構いません。
4 (bool Success, T Data) GetData();
5}
Successプロパティで成功の有無を判定する機能が盛り込まれています。これであれば、「取得失敗」は「例外的状況」ではなくなり、GetDataの動作パターンの一つとなるわけです。
実際、IGetDataをどちらの定義にするかは人によります。GetDataメソッドはデータを返すのが「当たり前」であるなら、返せないのは「例外である」としてしまっても何の問題も無いのです。その時は、GetDataメソッドを使用するDoメソッドの方で、「失敗した場合の例外処理」を記述しなければならないことになります。
個人的な感覚だと、GetDataメソッドは引数を持っていないので、「データを取得できるのが当たり前」であると認識できるため、データを取得できないのは「例外的状況である」としてしまいます。
そして、ParseDataメソッドはパースするデータを引数で入れているため、「引数によって成功するかどうかが決まる」となるため、戻り値に「成功したかどうか」の情報を盛り込みます。
次に、「異常時には終了」についてです。
まず、Doメソッドが「アプリケーション全体の終了を管理できる」というのは権限を持ちすぎです。
Doがやるのは「DataをGetしてParseしてSendする」だけであるべきで、取得できなかった場合の処理はDoメソッドが扱える範囲でしか記述してはいけません。アプリケーションの終了までを行いたいのであれば、各タスクの失敗をDoメソッドが認識した時点で例外を投げるしかないです。Doが「失敗したらアプリケーションを終了させたい」のであれば、それはDoの身に余る行為なので、こういう場合は予め定義しておいた独自例外をスローし、上位レイヤ（場合によってはエントリポイントまで遡る）でその例外をcatchし処理する方針になるはずです。このときのDoメソッドは、「ログを出力してから例外をスロー」よりは、エラーログを持たせる作りの独自例外を用意し、catchした場所で異常終了の事由をログに出力する形でも良いと思います。このエラーログは「アプリケーションがしぬほどの事由」に相当するので、それ相応のレイヤで扱うほうがスマートに見えるのです。
追記:01/20
想像でコード例を書いてみました。
IFixedLogがログの実体だと仮定しています。
部分的に適当なので、頑張って読み替えてください。
csharp
1static int Main() // エントリポイント
2{
3 try
4 {
5 test.Do();
6 return 0; // 正常終了
7 }
8 catch (FatalErrorException exception) // 致命的なエラーによる強制終了の場合は
9 {
10 var log = exception.Log; // 例外からLogを取り出す
11 logger.Write(log); // ログを出力
12 return -1;
13 }
14 catch (Exception exception) // 予期しない例外の場合は
15 {
16 var log = CreateLog("予期しないエラー" + exception);
17 logger.Write(log);
18 return -1;
19 }
20}
21
22// 独自例外
23class FatalErrorException : Exception
24{
25 public IFixedLog Log { get; } // 例外にLogを持たせる
26
27 public FatalErrorException(IFixedLog log) : base()
28 {
29 this.Log = log;
30 }
31}
32
33// 仮にこんなインターフェースで
34interface IGetData<T>
35{
36 // 必ずGetできる。失敗するのは例外的状況とする。
37 T GetData();
38}
39interface IParseData<TRaw, TParsed>
40{
41 // 与えるrawによっては失敗する。成功した場合はDataの中に結果が入る。
42 (bool Success, TParsed Data) ParseData(TRaw raw);
43}
44interface ISendData<T>
45{
46 // 失敗しない。
47 void Send(T data);
48}
49
50
51class TestClass<TData, TParsed>
52{
53 public Test(IGetData<TData> getter, IParseData<TData, TParsed> parser, ISendData<TParsed> sender)
54 {
55 // ...
56 }
57
58 public void Do()
59 {
60 var raw = default(TData);
61 try
62 {
63 raw = _getter.GetData(); // GetDataは基本成功
64 }
65 catch (GetDataException) // GetDataがGet失敗した時に出す例外「だけ」をcatch
66 {
67 var log = CreateLog("get失敗");
68 throw new FatalErrorException(log); // 致命的なエラーを例外で表現する
69 }
70
71 var parsed ＝ _parser.ParseData(raw); // ParseDataは例外を出さない
72
73 if (!parsed.Success) // parseが失敗した場合は
74 {
75 var log = CreateLog("parse失敗");
76 throw new FatalErrorException(log); // ログ入りスロー
77 }
78
79 _sender.Send(parsed.Data);
80 }
81}
Doメソッドが自分のやるべきことだけをこなして、それ以外は上位に投げればいいというコードになっているところに注目して下さい。
追記:01/21
自アプリケーションが管理する例外は必ずログを持つという構造にしてみました。
csharp
1// アプリケーション例外のベースとなる抽象例外クラス
2public abstract class MyNantokaApplicationException : Exception
3{
4 public IFixedLog Log { get; }
5
6 protected MyNantokaApplicationException(IFixedLog log) : base()
7 {
8 this.Log = log;
9 }
10
11 protected MyNantokaApplicationException(IFixedLog log, string message) : base(message)
12 {
13 this.Log = log;
14 }
15
16 protected MyNantokaApplicationException(IFixedLog log, string message, Exception inner) : base(message, inner)
17 {
18 this.Log = log;
19 }
20}
21
22// 例えば、アプリケーションを強制終了するほどの致命的なエラーを表す
23public class FatalErrorException : MyNantokaApplicationException
24{
25 public FatalErrorException(IFixedLog log) : base(log)
26 { }
27
28 public FatalErrorException(IFixedLog log, string message) : base(log, message)
29 { }
30
31 public FatalErrorException(IFixedLog log, string message, Exception inner) : base(log, message, inner)
32 { }
33}
34
35// GetDataがGetに失敗したときに出す例外とする
36public class GetDataException : MyNantokaApplicationException
37{
38 public GetDataException(IFixedLog log) : base(log)
39 { }
40}
41
42// Doメソッドの前半だけ
43public void Do()
44{
45 var raw = default(TData);
46 try
47 {
48 raw = _getter.GetData();
49 }
50 catch (GetDataException exception) // GetDataがGetに失敗したということしか分からない
51 {
52 var log = exception.Log; // GetDataExceptionから実際に失敗した理由のLogを取り出す
53 throw new FatalErrorException(log); // Get失敗は致命的なエラー扱いとして、そのログを入れてスロー
54 }
55
56 // ...
57}
58
59// ファイルを開くIGetDataのGetDataメソッドの例
60public TData GetData()
61{
62 try
63 {
64 /* ファイルオープン */
65 return data;
66 }
67 catch
68 {
69 var log = FileOpenErrorLog.Instance; // IFixedLogオブジェクトがこんな感じのシングルトンだと仮定
70 throw GetDataException(log); // ファイルオープン失敗のIFixedLog入りでGetDataExceptionをスロー
71 }
72}
73
74// メモリから読み込むIGetDataのGetDataメソッドの例
75public TData GetData()
76{
77 try
78 {
79 /* メモリ読み込み */
80 return data;
81 }
82 catch
83 {
84 var log = MemoryReadErrorLog.Instance;
85 throw GetDataException(log); // 読み込み失敗のIFixedLog入りでGetDataExceptionをスロー
86 }
87}
この方法であれば、Doが「Getが失敗したこと」までしか把握していない状況で「ファイルオープン失敗」のログを正しく吐かせることができます。
追記:01/24
MapData回りの再設計例です。
csharp
1// IParseDataのリネーム
2interface IConverter<in T, TResult>
3{
4 Tuple<bool, TResult> Convert(T data);
5}
6
7interface IValidateData<in T>
8{
9 bool ValidateData(T data);
10}
11
12class CollectionParser<TRawItem, TMappedItem> : IConverter<IEnumerable<TRawItem>, IEnumerable<TMappedItem>>
13{
14 private readonly IConverter<TRawItem, TMappedItem> _mapper; // 変数名はわかりやすさのため無変更で
15
16 public CollectionParser(IConverter<TRawItem, TMappedItem> mapper)
17 {
18 this._mapper = mapper;
19 }
20
21 public Tuple<bool, IEnumerable<TMappedItem>> Convert(IEnumerable<TRawItem> rawItems)
22 {
23 var mapped = rawItems
24 .Select(_mapper.Convert)
25 .Where(result => result.Item1) // 失敗をフィルタし、
26 .Select(result => result.Item2) // Dataを取り出し、
27 .ToArray(); // 配列化
28
29 return Tuple.Create(mapped.Length == rawItems.Count(), mapped.AsEnumerable());
30 }
31}
32
33// CollectionParserもSimpleMapperもどっちもIConverter。それぞれ意味は違うのでこうやって二重に使える。
34class XXXSimpleMapper : IConverter<string, Something>
35{
36 private readonly IValidateData<string> _validator; // validatorはMapperの中に持たせる
37
38 public XXXSimpleMapper(IValidateData<string> validator)
39 {
40 this._validator = validator;
41 }
42
43 public Tuple<bool, Something> Convert(string raw)
44 {
45 if (!this._validator.ValidateData(raw))
46 return Tuple.Create(false, default(Something));
47
48 // Validateが成功した以上、変換は必ず成功することを期待する。
49 // 検証したい項目を増やすなら、Validatorを増やせばいい。
50
51 var name = raw.Substring(0, 10).TrimStart();
52 var message = raw.Substring(10);
53
54 return Tuple.Create(true, new Something(name, message));
55 }
56}
57
58// Validatorは特に変更してない
59class XXXXLogDataValidatorWithLog : IValidateData<string>
60{
61 private readonly IFixedLogger _logger;
62
63 public XXXXLogDataValidatorWithLog(IFixedLogger logger)
64 {
65 this._logger = logger;
66 }
67
68 // ValidateDataは「Convert可能なデータである」ことを検証する。
69 // 例えば、Substringを使うConverterを想定するなら、Lengthのチェックも必要。別のValidatorを新たに実装しても良い。
70 public bool ValidateData(string data)
71 {
72 var result = data == string.Empty;
73 // 失敗したらログを出力
74 if (!result)
75 {
76 _logger.Log(UnavailableDataDetectedErrorLog.GetInstance(data, "文字列が空文字"));
77 }
78
79 return result;
80 }
81}
かなりシンプルかつ明瞭になったと思うんですが、どうでしょう？
追記:01/24_02
質問2のほうにも回答していきます。
まずは、質問内容以前の話なんですが。「ファイルは開始時にオープンして終了時にクローズすることになりました。」という設計がまずおかしいです。ファイルを常時オープンしっぱなしにする理由はなんでしょうか？外部リソースは「使用する時だけ開いて、すぐに閉じる」が安全に使用するための鉄則です。コードを見る限り、常時ファイルを開いておかなければならない理由は全く無いように見えます。終了時まで一切クローズしないのであれば、そもそもIDisposableの実装も冗長ということになります。質問のコードから読み取れない追加情報があったとしても、リソースの保持というのはかなり危ないコーディングなので、推奨しません。なんとかしてリソースの保持を行わないように可能な限り設計を見直します。
例えば、自分なら以下のように書きます。これ以外はないです。
csharp
1// データをファイルに送信するISendDataの実装
2class PlainFileSender : ISendData<IEnumerable<Something>>
3{
4 private readonly string _path;
5
6 public PlainFileSender(string path)
7 {
8 this._path = path;
9 }
10
11 public void SendData(IEnumerable<Something> data)
12 {
13 try
14 {
15 using (var writer = new StreamWriter(this._path, true))
16 {
17 // データをテキストファイルに追加
18 }
19 }
20 catch
21 {
22 var log = FileAppendErrorLog.Instance;
23 throw new SendDataException(FileAppendErrorLog.Instance);
24 }
25 }
26}
一応、質問のほう、コンストラクタで発生させる例外についても書きますね。
エントリポイントを丸ごとtryで囲んでいるんですよね？であれば、インスタンスの生成時に「アプリケーションを継続できない状態」が発生するなら、コンストラクタ内でログを作成してFatalErrorをスローしてやればいいです。例外に「処理の流れ」なんていうものはなくて、その例外的状況「アプリケーションを継続できない」に相当するほどの異常事態が発生した際に、自分の作業をその場でぶん投げるためにあるのです。例外を書くときに「処理フロー」を考え始めるとドツボにはまるので注意して下さい。「やってみてなんかダメそうだったからもう投げる」「自分はこの例外が来るパターンは知ってるから、catchする体制だけ整えてある」という、直接相手のことを意識しないコーディングを心がけると良いです。
Q&A
バッドをするには、ログインかつ
こちらの条件を満たす必要があります。
2017/01/20 08:18
2017/01/20 08:42
2017/01/20 09:32
2017/01/20 10:11
2017/01/21 02:13
2017/01/21 16:49 編集
2017/01/21 05:58
2017/01/21 07:30 編集
2017/01/21 13:04 編集
2017/01/21 17:18 編集
2017/01/22 01:12
2017/01/22 05:51 編集
2017/01/22 06:16
2017/01/22 06:44
2017/01/22 15:33 編集
2017/01/23 02:43
2017/01/23 14:57
2017/01/24 10:00 編集
2017/01/24 12:52 編集
2017/01/25 02:00
2017/01/27 01:27 編集
2017/01/27 02:02