Should I make sure an object is unusable after Dispose was called?-CodePudding

I have a class BleScanner that wraps an internal BluetoothLEAdvertisementWatcher. It also implements IDisposable to make sure that the watcher is stopped when the scanner gets disposed of.

public sealed class BleScanner : IDisposable
{
    public event AdvertisementReceivedHandler? AdvertisementReceived;

    private readonly BluetoothLEAdvertisementWatcher m_Watcher;

    public BleScanner() {
        m_Watcher = new() {
            // ...
        };
        // m_Watcher.Received  = OnAdvertisementReceived;
    }

    // private void OnAdvertisementReceived(...) {
    //    code elided for brevity
    //    may eventually raise AdvertisementReceived
    // }

    public void Start() => m_Watcher.Start();

    public void Stop() => m_Watcher.Stop();

    public void Dispose() {
        if (m_Watcher.Status == BluetoothLEAdvertisementWatcherStatus.Started) {
            m_Watcher.Stop();
        }
    }
}

The watcher is not disposable. So in theory, the scanner would still work if you just called Start again after Dispose:

public async Task ScannerTest(CancellationToken token) {
    using var scanner = new BleScanner();
    scanner.AdvertisementReceived  = OnAdvertisementReceived;

    scanner.Start(); // will start the scan
    await Task.Delay(3000, token); // raise events for 3 seconds
    scanner.Stop(); // could be forgotten
    scanner.Dispose(); // will stop the scan if indeed it was forgotten
    
    scanner.Start(); // everything will work, despite "scanner" being disposed already
}

Should I make sure Start (and maybe Stop) throws an ObjectDisposedException after Dispose was called? The guidelines on the Dispose pattern only require that Dispose can be called multiple times without an exception, but don't say anything about how the other members should behave after Dispose was called. Neither does using disposable objects of the IDisposable interface say what to expect when calling methods on a disposed object.

CodePudding user response：

In your question, you reference the IDisposable Guidelines. The first line says "Implementing the Dispose method is primarily for releasing unmanaged resources." I don't think that's what you're doing here. If BluetoothLEAdvertisementWatcher was IDisposable, then you could dispose of it in your Dispose() function; but that isn't the case. So, garbage collection will take care of your object in its sweet time after your object falls out of scope; just let it do its thing.

Hope that helps.

CodePudding user response：

It's totally fine to use IDisposable to free managed resources, which is really any kind of scope that requires code to be run at the end of that scope.

In this case, I would say that the scope is really around Start and Stop. So I would have Start return an IDisposable that calls Stop (and make Stop private). Your type would not be disposable. E.g., using Disposable from my Nito.Disposables library:

public sealed class BleScanner
{
    public event AdvertisementReceivedHandler? AdvertisementReceived;

    private readonly BluetoothLEAdvertisementWatcher m_Watcher;

    public BleScanner() {
        m_Watcher = new() {
            // ...
        };
        // m_Watcher.Received  = OnAdvertisementReceived;
    }

    public void Start()
    {
        m_Watcher.Start();
        return Disposable.Create(() => Stop());
    }

    private void Stop() => m_Watcher.Stop();
}

public async Task ScannerTest(CancellationToken token) {
    var scanner = new BleScanner();
    scanner.AdvertisementReceived  = OnAdvertisementReceived;

    using var scannerSubsctiption = scanner.Start();
    await Task.Delay(3000, token); // raise events for 3 seconds
}