.NET Core Dispose implementation

Question

I'm a little confused how to implement Dispose pattern correctly.

I have a class:

public class DomainActions : IDomainActions, IDisposable
{
    private HttpClient _client;

    public DomainActions(string baseAddress, string token)
    {
        _client = new HttpClient();
        _client.BaseAddress = new Uri(baseAddress);
        _client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token);

    }

    public async Task<List<DomainDto>> GetDomainListAsync()
    {
        var requestStream = await _client.GetAsync("domains");
        var requestString = await requestStream.Content.ReadAsStringAsync();

        return ParseDomainListResponse(requestString);

    }

as I understand, I have to allocate _client

public class DomainActions : IDomainActions
{
    //...
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    private bool _disposed = false;
    public virtual void Dispose(bool disposing)
    {
        if (_disposed) return;
        if (disposing)
        {
            if (_client != null)
            {
                _client.Dispose();
                _client = null;
            }
        }
        _disposed = true;
    }
}

my implemetation is correct, is it enough?

Since `_client` is of a type that implements `IDisposable`, you need to call `Dispose` on it before setting it to null — vc 74, Jan 08 '19 at 15:47
It looks fine though. You can have a look at msdn guide here https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose — fhnaseer, Jan 08 '19 at 15:51
https://stackoverflow.com/questions/15705092/do-httpclient-and-httpclienthandler-have-to-be-disposed — Daniel A. White, Jan 08 '19 at 16:06
@OlegSh I this case I'd definitely dispose of the HttpClient when disposing the instance since it is never going to be used again after disposal (unlike a global client that you could use for subsequent queries) — vc 74, Jan 08 '19 at 19:39

Yennefer · Accepted Answer · 2020-11-04T17:29:46.310

You are not showing us where you create _client, but let's figure out it is done in the constructor.

You are disposing the object correctly. For reference, this is the Visual Studio snippet for a complete disposal:

    #region IDisposable Support
    private bool disposedValue = false; // To detect redundant calls
    
    protected virtual void Dispose(bool disposing)
    {
        if (!disposedValue)
        {
            if (disposing)
            {
                // TODO: dispose managed state (managed objects).
            }

            // TODO: free unmanaged resources (unmanaged objects) and override a finalizer below.
            // TODO: set large fields to null.

            disposedValue = true;
        }
    }
    
    // TODO: override a finalizer only if Dispose(bool disposing) above has code to free unmanaged resources.
    // ~A() {
    //   // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
    //   Dispose(false);
    // }

    // This code added to correctly implement the disposable pattern.
    public void Dispose()
    {
        // Do not change this code. Put cleanup code in Dispose(bool disposing) above.
        Dispose(true);
        // TODO: uncomment the following line if the finalizer is overridden above.
        // GC.SuppressFinalize(this);
    }
    #endregion

If your unmanaged resources do not require special cleanup and can be represented by handles, you should use SafeHandles which are less error prone and more concise. For further information, please have a look at Implement a Dispose Method.

If you have unmanaged resources: the docs (https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose) advise to construct System.Runtime.InteropServices.SafeHandle objects instead of implementing a finalizer — Dejan, Nov 04 '20 at 10:52
That is true if you can collapse your resources by mere handles. There are cases which require you to call additional methods to perform the cleanup. I posted the Dispose pattern to show them how it was supposed to work. I improved the answer. Thank you:) — Yennefer, Nov 04 '20 at 17:30
Not only does their object not have unmanaged resources that can't be represented by a safe handle, *they don't have any unmanaged resources at all*. None of this is helpful and is even *detrimental* to include in a situation such as this. — Servy, Nov 04 '20 at 18:05

.NET Core Dispose implementation

1 Answers1