CODEWITHSUNDEEP
c#.netdispose
JD Davis
JD Davis

Reputation: 3720

Only dispose of initialized objects?

I have an API wrapper that has a series of properties that are initialized on their first use. When the class's Disposed method is called, it disposes of all of the other classes it's storing. However, there is a good chance only one or two of these other classes have been initialized anyways. If these classes aren't already initialized, the Dispose method actually ends up initializing them and them disposing them. This is massively resource and time intensive, but I cannot think of a method to remedy it.

Here's the class.

public class SalesForceApi : ISalesForce, IDisposable
{
    public Logger Logger = LogManager.GetCurrentClassLogger();

    private IAccounts _accounts;
    private IAttachments _attachments;
    private ICases _cases;
    private IContacts _contacts;
    private IGroups _groups;
    private IRecordTypes _recordTypes;
    private IUsers _users;

    public IAccounts Accounts => _accounts ?? (_accounts = new Accounts());

    public IAttachments Attachments => _attachments ?? (_attachments = new Attachments());
    public ICases Cases => _cases ?? (_cases = new Cases());
    public IContacts Contacts => _contacts ?? (_contacts = new Contacts());
    public IGroups Groups => _groups ?? (_groups = new Groups());
    public IRecordTypes RecordTypes => _recordTypes ?? (_recordTypes = new RecordTypes());
    public IUsers Users => _users ?? (_users = new Users());

    public SalesForceApi()
    {

    }

    public void Dispose()
    {
        Logger.Trace("Disposing of Acccounts...");
        Accounts.Dispose();
        Logger.Trace("Disposing of Attachments...");
        Attachments.Dispose();
        Logger.Trace("Disposing of Cases...");
        Cases.Dispose();
        Logger.Trace("Disposing of Contacts...");
        Contacts.Dispose();
        Logger.Trace("Disposing of Groups...");
        Groups.Dispose();
        Logger.Trace("Disposing of RecordTypes...");
        RecordTypes.Dispose();
        Logger.Trace("Disposing of Users...");
        Users.Dispose();
    }
}

And here is what the resulting log looks like:

SalesForceApi_v1.SalesForceApi::Disposing of Acccounts...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of Attachments...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of Cases...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of Contacts...
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of Groups...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of RecordTypes...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.
SalesForceApi_v1.SalesForceApi::Disposing of Users...
SalesForceApi_v1.Requests.RequestBase::Logging into SalesForce...
SalesForceApi_v1.Requests.RequestBase::SalesForce login successful!
SalesForceApi_v1.Requests.RequestBase::Logging out of SalesForce.

Upvotes: 0

Views: 842

Answers (3)

Sam
Sam

Reputation: 10113

You can wrap your disposal in a private member null check. Since you are lazy loading, calling Property.Dispose() actually creates an instance of the class, just to dispose of it. I also like to set my members to null after disposal so that it does throw a NullReferenceException if I try to access it again.

public void Dispose()
{
    if (_accounts != null) 
    {
        Logger.Trace("Disposing of Acccounts...");
        Accounts.Dispose();
        // or
        // _accounts.Dispose();
        // _accounts = null;
    }
    if (_attachments != null) 
    {
        Logger.Trace("Disposing of Attachments...");
        Attachments.Dispose();
        // or
        // _attachments.Dispose();
        // _attachments = null;
    }
    if (_cases != null) 
    {
        Logger.Trace("Disposing of Cases...");
        Cases.Dispose();
        // or
        // _cases.Dispose();
        // _cases= null;
    }
    if (_contacts != null) 
    {
        Logger.Trace("Disposing of Contacts...");
        Contacts.Dispose();
        // or
        // _contacts.Dispose();
        // _contacts = null;
    }
    if (_groups != null) 
    {
        Logger.Trace("Disposing of Groups...");
        Groups.Dispose();
        // or
        // _groups.Dispose();
        // _groups = null;
    }
    if (_recordTypes != null) 
    {
        Logger.Trace("Disposing of RecordTypes...");
        RecordTypes.Dispose();
        // or
        // _recordTypes.Dispose();
        // _recordTypes = null;
    }
    if (_users!= null) 
    {
        Logger.Trace("Disposing of Users...");
        Users.Dispose();
        // or
        // _users.Dispose();
        // _users= null;
    }
}

Upvotes: 0

Jakub Lortz
Jakub Lortz

Reputation: 14894

Don't use properties in the Dispose method, use private fields.

public void Dispose()
{
    Logger.Trace("Disposing of Acccounts...");
    if(_accounts != null) _accounts.Dispose();
    Logger.Trace("Disposing of Attachments...");
    if(_attachments != null) _attachments.Dispose();
    // ...
}

Upvotes: 3

adv12
adv12

Reputation: 8551

When disposing, don't call the methods that lazy-init; test and dispose the private member variables (_accounts, etc.).

Upvotes: 5

Related Questions