C# リファクタリング foreachで抽出
List
public void Hoge() { var list = new List<String>(); list.Add("123"); list.Add("1234"); list.Add("12345"); list.Add("123456"); // 4文字以下のものをリストから抽出したい。 var list2 = new List<string>(); foreach(var str in list) { if (str.Length <= 4) list2.Add(str); } }
foreachをなるべく書かないようにする。
public void Hoge() { var list = new List<String>(); list.Add("123"); list.Add("1234"); list.Add("12345"); list.Add("123456"); // 4文字以下のものをリストから抽出したい。 var list2 = new List<string>(); list2 = list.FindAll(x => x.Length <= 4); }
さらに行数を減らすなら
public void Hoge() { var list = new List<String>(); list.Add("123"); list.Add("1234"); list.Add("12345"); list.Add("123456"); // 4文字以下のものをリストから抽出したい。 var list2 = new List<string>(list.FindAll(x => x.Length <= 4)); }
コンストラクタでそのまま渡してしまう。
だた括弧がネストしすぎると読みにくいし、デバッグ時にステップ実行しにくいという問題もある。
個人的にはステップ実行で動作確認するくらいならテストコード書けば良いと思いますが。
FindAllメソッドをRemoveメソッドに変えた場合
public void Hoge() { var list = new List<String>(); list.Add("123"); list.Add("1234"); list.Add("12345"); list.Add("123456"); // 4文字以下のものをリストから抽出したい。 list.RemoveAll(x => x.Length >= 5); }
FindAllメソッドとの違いとしては
・FindAllメソッドは戻り値がList
・RemoveAllメソッドは実行したコレクション自体を編集し、戻り値がint
C# リファクタリング コレクションを返すメソッド
なんらかの処理を行ってコレクションを返すようなメソッドがあったとして、
こんな具合にパラメータがnullだったら例外を投げ返すんだったら
public IEnumerable<object> Hoge(object obj) { if (obj == null) throw new ArgumentNullException("obj"); // なにか処理してコレクションを返す。 }
こんな具合に空の配列を返してやる。
public IEnumerable<object> Hoge(object obj) { if (obj == null) new object[] { }; // なにか処理してコレクションを返す。 }
メソッドの目的にもよるのだろうけど、
例えば検索処理とかだと変なパラメータを渡したら例外を投げずに空の結果を返してやれば良い。
書いてみて思ったけど、おかしなことをやっているならやっぱり例外を投げて気づく機会を設けるべきなんだろうか・・・
C# リファクタリング 早期リターン
int型の引数が5以下だったらTrueを返すとして、
public bool Hoge(int value) { if(value < 5) { return true; }else{ return false; } }
こうだったのを
public bool Hoge(int value) { if (value < 5) return true; return false; }
こうする。
ソースコードが5行から2行へ、
ごくごくシンプルな例ですが、複雑な条件が組み合わさる場合時は、
結果が確定している時点でさっさとメソッドから抜け出すようにすると修正時のリスクが少し軽減出来る。
オブジェクト指向もといプログラムを考える。
レガシープログラマの歴が長く、オブジェクト指向は独学、
何が良いプログラムで何が悪いプログラムなのかもやもやする・・・
日々頭の中の考えがあっちいったりこっちいったりで切りがないので、
気が付いた時に文章に書いておいた方がいいかもしれない。
今日思ったこと。
プログラムを作る時、まずごくごくシンプルなインターフェースで考える。
インターフェースが決まったらインターフェースをコンポジットする実装クラスを作る。
そして単一の処理を行う実装クラスを作る。
後はユースケースに合わせてコンポジットしたものを用意する。
同一モデルに対するチェック処理や手続き的な編集処理はこういう作りにしとくと組み換えが利いて良いかもしれない。
C# ドメインイベントの実装 つづき
前回のエントリーの続きから
nomoredeathmarch.hatenablog.com
Publisherクラスのメソッドについて記述
まずSubscribeメソッドから
public static void Subscribe(ISubscriber subscriber) { try { Mutex.WaitOne(); if (subscriber == null) throw new ArgumentNullException("subscriber"); if (Subscribers.Contains(new WeakReference<ISubscriber>(subscriber))) return; Subscribers.Add(new WeakReference<ISubscriber>(subscriber)); } finally { Mutex.ReleaseMutex(); } }
・イベントの通知を予約するためのメソッド
・Mutexクラスを使って排他制御(finallyで解放を忘れずに)
・イベントの多重処理防止のため登録済みのサブスクライバだったら処理を抜ける。
・サブスクライバの弱参照をリストに追加する。
続いてUnsubscribeメソッド
public static void Unsubscribe(ISubscriber subscriber) { try { Mutex.WaitOne(); if (subscriber == null) throw new ArgumentNullException("subscriber"); Subscribers.RemoveAll(x => subscriber.Equals(ToStrong(x))); } finally { Mutex.ReleaseMutex(); } }
・イベントの通知予約を解除するメソッド。
・Subscribeメソッドと同様にMutexで排他制御
・リストからの削除は強い参照に変換して対象を比較
続いてToStrongメソッド
private static ISubscriber ToStrong(WeakReference<ISubscriber> obj) { ISubscriber ret; obj.TryGetTarget(out ret); return ret; }
・弱参照を強参照に変換して返すメソッド。
・クラス内の他のメソッド内で呼び出されるprivateなメソッド。
最後にPublishメソッド
public static void Publish(IEvent e) { try { Mutex.WaitOne(); if (e == null) throw new ArgumentNullException("e"); var list = new List<ISubscriber>(); list.AddRange(Subscribers.ConvertAll<ISubscriber>(x => ToStrong(x))); list.RemoveAll(x => x == null || !x.CanHandle(e)); list.ForEach(x => x.Handle(e)); Subscribers.RemoveAll(x => ToStrong(x) == null); } finally { Mutex.ReleaseMutex(); } }
・受け取ったイベントをサブスクライバに通知する。
・ここでもやっぱりMutexで排他制御
・弱参照のリストを強参照に変換、変換出来なった要素やCanHandleメソッドでFalseが返る要素をリストから除外
・ForEachメソッドで各要素のHandleメソッドにイベントを渡す。
・イベントの通知が終わったらGC回収済みのサブスクライバをリストから除外する。
注意事項
1.排他制御
今回はMutexクラスを使って各Publicメソッドの先頭で排他制御を掛けた。
マルチスレッド周りのプログラムは正直あまり理解していないので辛うじて自分が分かる方法です・・・
2.メモリリーク対策
静的なリストはRemoveなりClearメソッドで中身を消さないと、アプリケーションが動いている限り参照が残り続け、メモリリークを起こす。クライアントコード側でDisposeを実装し、確実にリストから排除する仕組みになっていれば問題ないが、実装漏れが起こるリスクを考えるとPublisherクラス内を弱参照を駆使して実装する方が安全と言える。
3.購読停止の管理
サブスクライバがGCに回収されない限りイベントは通知される。購読が不要になった時点でUnsubscribeメソッドを呼び出す。
C# ドメインイベントの実装
こちらの書籍の第8章で紹介されているパターン
通常のイベントハンドラを使わずにクラスをイベントに見立て、クラス間の結合を弱める。
- 作者: ヴァーン・ヴァーノン
- 出版社/メーカー: 翔泳社
- 発売日: 2015/03/19
- メディア: Kindle版
- この商品を含むブログ (2件) を見る
偶然なのか、こちらの書籍でも第8章
.NETのエンタープライズアプリケーションアーキテクチャ第2版 .NETを例にしたアプリケーション設計原則
- 作者: ディノエスポシト,アンドレアサルタレロ
- 出版社/メーカー: 日経BP社
- 発売日: 2015/06/18
- メディア: Kindle版
- この商品を含むブログを見る
自分なりに実装
まずIEventインターフェース
namespace Nmdm.Events { public interface IEvent { } }
これはイベントである目印をつけるためのマーカー
続いて、ISubscriberインターフェース
namespace Nmdm.Events { public interface ISubscriber { void Handle(IEvent value); bool CanHandle(IEvent value); } }
こちらはIEventを受け取って処理を行うためのインターフェース
HandleメソッドはIEventを引数に処理を行うメソッド、
CanHandleメソッドはIEventを処理できるかどうかを後述のPublisherに伝えるためのメソッド
続いてPublisher、そのままだと長いのでメソッドの中身は後述
using System; using System.Collections.Generic; using System.Threading; namespace Nmdm.Events { public static class Publisher { private static Mutex Mutex { get; } = new Mutex(); private static List<WeakReference<ISubscriber>> Subscribers { get; } = new List<WeakReference<ISubscriber>>(); public static void Publish(IEvent e) { //省略 } public static void Subscribe(ISubscriber subscriber) { //省略 } public static void Unsubscribe(ISubscriber subscriber) { //省略 } private static ISubscriber ToStrong(WeakReference<ISubscriber> obj) { //省略 } } }
Publisherクラスは受け取ったIEventをISubscriberに通知するのが役割。
クラスメンバーについて補足:
・Mutex⇒スレッドセーフにするための排他制御用
・Subscribers⇒ISubscriberの弱参照リスト、IEventの通知先保存用
・Publish(IEvent e)⇒Subscribersに受け取ったイベントを通知する。
・Subscribe(ISubscriber subscriber)⇒受け取ったサブスクライバを弱参照リストに追加する。
・Unsubscriber(ISubscriber subscriber)⇒受け取ったサブスクライバを弱参照リストから削除する。
・ToStrong(WeakReference
とりあえず今日はここまで。