CA1001 Visual Studio 2012 Code Analysis warning. What does it mean?

Question

It is not that important but I am trying to figure out what it is telling me and is it a legitimate warning ? Can someone explain this error in simple terms for me ?

CA1001 Types that own disposable fields should be disposable

Implement IDisposable on 'MemVoteManager' because it creates members of the following IDisposable types: 'CongressDBEntities'. If 'MemVoteManager' has previously shipped, adding new members that implement IDisposable to this type is considered a breaking change to existing consumers.

    public class MemVoteManager : AbstractDataManager, IMemVoteManager
{
    private CongressDBEntities context = new CongressDBEntities();

    public int AddMemVote(tMemVoteScore mvs)
    {
        //Insert Model
        context.tMemVoteScores.Add(mvs);
        context.SaveChanges();

        int newPK = mvs.MemVoteScoresID;

        //Update funky column ID with PK as well
        var memVoteItem = (from m in context.tMemVoteScores
                           where m.MemVoteScoresID == newPK
                           select m).SingleOrDefault();

        memVoteItem.ID = memVoteItem.MemVoteScoresID;
        context.SaveChanges();
        return newPK;
    }

Solution 1

You could implement IDisposable so the context will be disposed of when consumers are done with your class, but you may be better off NOT having the context be a member of the class. Just create it when you need it and dispose of it when you're done:

public int AddMemVote(tMemVoteScore mvs)
{
    //Insert Model
    using(CongressDBEntities context = new CongressDBEntities())
    {
        context.tMemVoteScores.Add(mvs);
        context.SaveChanges();

        int newPK = mvs.MemVoteScoresID;

        //Update funky column ID with PK as well
        var memVoteItem = (from m in context.tMemVoteScores
                           where m.MemVoteScoresID == newPK
                           select m).SingleOrDefault();

        memVoteItem.ID = memVoteItem.MemVoteScoresID;
        context.SaveChanges();
    }
    return newPK;
}

Contexts are lightweight so there's not a huge penalty for creating them each time. Plus you then don't have to worry about consumers notifying you to dispose of the context, and you don't have a lot of built-up changes in memory if one instance of the class is used many times.

Solution 2

It's letting you know that field context contains disposable members. That means those members need to have Dispose() called on them so that Garbage Collection can occur. Therefore, it wants you to implement the interface IDisposable on MemVoteManager so that you can call Dispose() on the context and/or its members that are disposable.

So modify you code as such:

public class MemVoteManager : AbstractDataManager, IMemVoteManager, IDisposable

and then implement the members of the IDisposable interface like this:

public void Dispose()
{
    // call dispose on the context and any of its members here
}

Comments

[punkouter]

But I thought everything was eventually garbage collected after it went out of scope? But ill go ahead and wrap it like you did above if that will make code analysis happy

[punkouter]

So any class inside my class that implements Idisposable means I need to implement iDisposable as well? Without it can I create a memory leak ? Like I mentioned above I though everything is garbage collected.. its managed code!

[Mike Perrenoud]

@punkouter, if the class implements IDisposable that explicitly means that it has unmanaged resource to get rid of. A database connection for example (the actual connection) - that's an unmanaged resource. So, since you have a context object, which has other objects that eventually lead to unmanaged resources, you need to make sure that you get Dispose() called on them.

[Mike Perrenoud]

@punkouter, the problem with that thought process is that there are a number of ways objects never reaches garbage collection. Memory management, though it's automated in .NET, isn't thoughtless. Consider a class that has a reference to another class that is also referenced by an object that lives forever. I know it seems like an edge case - but it's not. None of those objects will be garbage collected until the application shuts down.

[punkouter]

Ok. So implementing I Disposable and calling it is a way to be sure that the class will be garbage collected... Maybe it is not needed now but it is best practice to always implement it anyways just incase

[punkouter]

but wrapping the DBcontext with a 'using' will be the same as implementing an idisposable ?

[Mike Perrenoud]

@punkouter, that is correct, but that's also assuming you pull it out of that class and use it directly.

[D Stanley]

@punkouter well, the context will be garbage collected either way. Implementing IDisposable gives you control over when certain resources are freed up. For example, disposing of the context closes the underlying database connection. If you waited for garbage collection, you have no control over when the connection is closed.