2011-05-27 2 views
2

以下のコードスニペットで意見を聞きたいと思います。改善できるものはありますか?ベストプラクティスに従って、イベントハンドラ/ライザーの名前が付けられていますか?私はそれが同じクラスのイベントを処理して上げることはそれほど有用ではないことを知っていますが、これは単なるスニペットです。イベントハンドラ/レイズコードスニペット

public class MyControl 
{ 
    public MyControl() 
    { 
     this.LogWritten += this.HandleMyControlLogWritten; 
    } 

    // Event handler 
    void HandleMyControlLogWritten(object sender, EventArgs e) 
    { 
    } 

    // Event object 
    public event Action<object, EventArgs> LogWritten; 

    // Event raiser 
    protected virtual void OnLogWritten(EventArgs e) 
    { 
     if (this.LogWritten != null) 
     { 
      this.LogWritten(this, e); 
     } 
    } 
} 

答えて

6

私がお勧めしたい主な変更点は、イベントハンドラのコピーを取得することです:あなたは(最終的に)マルチでこのクラスを使用することを計画している場合

// Event raiser 
protected virtual void OnLogWritten(EventArgs e) 
{ 
    var handler = this.LogWritten; 
    if (handler != null) 
    { 
     handler(this, e); 
    } 
} 

これは重要です - スレッドのシナリオ。そのように、私はそれが良い "ベストプラクティス"使用の習慣に入ることがわかります。問題は、コピーを作成せずに複数のスレッドで使用すると、接続されている「ハンドラ」のみがヌルチェックと呼び出しの間でサブスクライブ解除され、実行時エラーが発生する可能性があるということです。一時変数(var handler = this.LogWritten;)行にコピーすると、サブスクライバリストの「スナップショット」を効果的に作成できます。次にはnullをチェックし、必要に応じて起動します。

他の変更は、イベント宣言自体にあります。代わりにAction<T1,T2>を使用する:

// Event object 
public event Action<object, EventArgs> LogWritten; 

私は(標準EventArgsの場合)またはEventHandler(カスタムEventArgsサブクラスを使用する場合)EventHandler<TEventArgs>を使用することをお勧めします。これらはもっと「標準的なプラクティス」であり、他の開発者が期待するものになります。

// Event object 
public event EventHandler LogWritten; 
+2

私は同意していますが、それはおそらく説明する価値があります*なぜ*それは良い考えです。 –

+0

@ジョン:ええと、私は当時にそれを書いていた;) –

+0

デリゲートを呼び出す間にデリゲートの呼び出しリストを変更することはできないと忘れていた。スリックリマインダー。 –