6

I am using a fairly simple DI pattern to inject my data repository into my controller classes, and I'm getting the CA2000 code analysis warning (Dispose objects before losing scope) on every one of them. I know why the warning is happening, and can usually figure out how to fix it, but in this case I cannot figure out

  1. How there is any possibility of an exception being thrown in between object creation and the method returning, or
  2. Where I can put try/finally blocks to get rid of the error.

Before I just give up and suppress the warning messages all over the place, is there a better way to achieve this same effect that doesn't result in a potential undisposed object?

public class AccountController : Controller
{
    public AccountController () 
        : this(new SqlDataRepository()) 
    {
    }

    public AccountController ( IDataRepository db )
    {
        this.db = db ?? new SqlDataRepository();

        // Lots of other initialization code here that I'd really like
        // to avoid duplicating in the other constructor.
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing && (this.db != null))
        {
            IDisposable temp = this.db as IDisposable;
            if (temp != null)
            {
                temp.Dispose();
            }
        }
    }
 }
Michael Edenfield
  • 28,070
  • 4
  • 86
  • 117
  • 2
    In any case, the `(this.db != null)` in the `Dispose` method is redundant as you do a null check on it again a few lines down. You may also, at the bottom of your `Dispose` method call `base.Dispose(disposing)` to make sure the `Controller`'s `Dispose` is properly cleaning up as well. – Jesse C. Slicer Dec 21 '12 at 16:49

2 Answers2

1

If you are using ASP.Net MVC, you can have your controller implement IDisposable, and the pipeline will take care of disposing it for you. See ASP MVC: When is IController Dispose() called?.

Community
  • 1
  • 1
Steve Czetty
  • 6,147
  • 9
  • 39
  • 48
  • `Controller` already implements `IDisposable` so I assumed that was already happening; at any rate, re-implementing `IDisposable` explicitly has no effect. – Michael Edenfield Dec 21 '12 at 16:29
  • 1
    The static analysis may not be able to tell that the disposal is taking effect, but you should be safe to ignore the error, because it is getting disposed. – Steve Czetty Dec 21 '12 at 16:36
  • @SteveCzetty - Safely ignoring the warning is not the issue. If I safely ignored every warning, my output screen would be dozens of pages long. We go through StyleCop and FxCop to remove all that noise so that when something is really wrong, it shows up and is easily read when we compile. – Quark Soup Oct 04 '19 at 18:52
0

Your repository implements IDisposable. Make your controller also implement IDisposable, and in the dispose method clean up your repository.

Arseny
  • 7,251
  • 4
  • 37
  • 52
Ross Dargan
  • 5,876
  • 4
  • 40
  • 53