2

最近、選択プロセスの一環としてクラスを実装するように依頼されました。私は要求通りにプログラムを行いました。しかし、私はテストに失敗しました。私は自分のソリューションの何が悪いのか知りたいと思っています。どんな助けでも大歓迎です。質問と私の解決策を以下に示します

質問：

構築から毎秒イベントを発生させるスレッドセーフクラスを実装します。経過秒数を求める関数が必要です。このクラスはIDisposableを実装する必要があり、disposeの呼び出し後の秒経過関数の呼び出しは失敗するはずです。

私の解決策：

namespace TimeCounter
{
public delegate void SecondsElapsedHandler(object o, EventArgs e);
/// <summary>
/// Summary description for SecondCounter
/// </summary>
public class SecondCounter : IDisposable
{
    private volatile int nSecondsElapsed;
    Timer myTimer;
    private readonly object EventLock = new object();
    private SecondsElapsedHandler secondsHandler;
    public SecondCounter()
    {
        nSecondsElapsed = 0;
        myTimer = new Timer();
        myTimer.Elapsed += new ElapsedEventHandler(OneSecondElapsed);
        myTimer.Interval = 1000;
        myTimer.AutoReset = false;
        myTimer.Start();
    }

    public void OneSecondElapsed(object source, ElapsedEventArgs e)
    {
        try
        {
            SecondsElapsedHandler handlerCopy;
            lock (EventLock)
            {
                handlerCopy = secondsHandler;
                nSecondsElapsed++;

            }
            if (secondsHandler != null)
            {
                secondsHandler(this, e);
            }
        }
        catch (Exception exp)
        {
            Console.WriteLine("Exception thrown from SecondCounter OneSecondElapsed " + exp.Message);
        }
        finally
        {
            if (myTimer != null)
            {
                myTimer.Enabled = true;
            }
        }
    }

    public event SecondsElapsedHandler AnotherSecondElapsed
    {
        add
        {
            lock (EventLock)
            {
                secondsHandler += value;
            }
        }
        remove
        {
            lock (EventLock)
            {
                secondsHandler -= value;
            }

        }
    }

    public int SecondsElapsed()
    {
        if (this.IsDisposed)
        {
            throw new ObjectDisposedException("SecondCounter");
        }
        return nSecondsElapsed;

    }

    private bool IsDisposed = false;
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
    private void Dispose(bool Disposing)
    {
        if (!IsDisposed)
        {
            if (Disposing)
            {

            }
            if (myTimer != null)
            {
                myTimer.Dispose();
            }

        }
        secondsHandler = null;
        IsDisposed = true;

    }
    ~SecondCounter()
    {
        Dispose(false);
    }
}
}
4

2 に答える 2

2

いくつかの問題があります：

  1. スレッドの問題とは特に関係ありませんが、一般的な例外の飲み込みに対してペナルティが科せられている可能性があります。

  2. タイマーに競合状態があります。タイマーが再び有効に設定される前にタイマーを破棄すると、例外が発生する可能性があるため、破棄します。

  3. DisposeでmyTimerをnullに設定することはありません。

  4. ファイナライザー（disposed = false）からマネージドクラスmyTimerにアクセスしていますが、これは悪い考えです。

  5. ロックを使用したイベントの明示的な実装は不要です。デリゲートは不変であり、イベントを追加/削除しても無効なデリゲート状態になることはありませんが、コールバックが発生するのとほぼ同時にデリゲートが追加/削除されると競合状態が発生する可能性があります。明示的なバッキングプライベートデリゲートなしで標準の「パブリックイベント」宣言を使用する場合、同期は自動的に処理されます。

  6. protected virtual（マイナーポイント）完全なDisposeパターンを実装している場合、派生クラスが破棄メカニズムにフックできるように、Dispose（bool disposed）メソッドをとしてマークするのが通例です。さらに良いことに、クラスにマークを付けるsealedと、ファイナライザーを完全に排除できます。

于 2010-11-02T21:05:43.860 に答える
0

ファイナライザーが壊れている可能性があります。パラメータとして正しく渡さfalseれDisposingます。これは、Dispose(bool)他の管理対象オブジェクトを破棄しようとしないように指示する必要があります。しかし、その方法では、次のように記述します。

if (Disposing)
{

}
if (myTimer != null)
{
    myTimer.Dispose();
}

したがって、の値は無視しますDisposing。Disposeこれは、オブジェクトがすでにファイナライズされている可能性がある場合に、ファイナライザースレッドからタイマーのメソッドを呼び出すことを意味します（ファイナライザーがある場合は、おそらくそうなります）。ファイナライザーは予測できない順序で実行されます。通常、ファイナライザーから他のGC管理対象オブジェクトを呼び出さないことをお勧めします。

実際、最近はファイナライザーをまったく作成しないことをお勧めします。質問はあなたにそれを書くように頼みませんでした！残念ながら、ほとんどのチュートリアルでIDisposableファイナライザーについても説明されています。彼らは異なる主題です。

Exceptionまた、ユニバーサル例外基本クラスをキャッチします。これは、のようなものをキャッチすることを意味しますNullReferenceException。通常は良い考えではありません。また、コンソールにログインします。これは、GUIまたはサーバーベースのアプリケーションではあまり価値がありません。

置き換えることができます：

myTimer.Elapsed += new ElapsedEventHandler(OneSecondElapsed);

と：

myTimer.Elapsed += OneSecondElapsed;

変数の命名に一貫性がありません。Microsoftのガイドラインを参照してください。

于 2010-11-02T21:12:49.647 に答える