Pregunta

I am not quite sure of what the proper way to Dispose my ObjectContext is. Here is how I am setup:

public abstract class DataManagerBase<T> where T:ObjectContext
{
    protected T _context = null;

    public string Message { get; set; }

    public DataManagerBase(T context)
    {
       _context = context;
    }
}

To use this in other classes, I am doing something like:

public class Test : DataManagerBase<DataEntities>
{
     public Test(DataEntities context) 
        : base(context){}

     public void InsertRecord(Person p)
     {
         if(_context != null)
         {
             try
             {
                 //Do insert logic
             }
             catch(Exception ex)
             {

             }
         }
    }

}

I have other methods that use the same _context, so I am not using a using statement, so should I check if the _context is not null if an exception is thrown and then dispose it? Basically I want to ensure that the _context is disposed when I am done with it, whether there is an exception or not. Would adding a finally to every try/catch be incorrect?

Would adding this method to my DataManagerBase class and then calling it in other classes do the trick:

 public void DisposeContext()
 {
        if (_context != null)
        {
            _context.Dispose();
        }
 }
¿Fue útil?

Solución

The rule of the thumb is: do not Dispose of objects which you did not create. So you should not dispose your context in any of the classes you provided to begin with.

You should however call context.Dispose() in class, where you actually create this particular instance. And when you do - there should be no other object using it. What you are trying to do is to avoid design issue you have instead of fixing it. Thats wrong, imho. And that will bite you back at some point, when you'll forget to put yet another null check somewhere.

Using the code you provided the example might look like:

void SomeMethodInOuterScope()
{
    var context = new DataEntities();

    var test = new Test(context);

    try
    {
        test.InsertRecord(new Person());
        ...............
    }
    catch(Exception ex)
    {
        ....
    }
    finally
    {
        context.Dispose();
    }
}

Here is another example (when context is not local)

class SomeClassInOuterScope : IDisposable
{
    private DataEntities _context;

    public SomeClassInOuterScope()
    {
        _context = new DataEntities();
    }

    public void Test()
    {
        var test = new Test(_context);
        test.InsertRecord(new Person());
        ...............
    }

    public void Dispose()
    {
        _context.Dispose();
    }
}

void SomeMethodInOuterOuterScope()
{
    var someclass = new SomeClassInOuterScope();

    try
    {
        someclass.Test();
        ...............
    }
    catch(Exception ex)
    {
        ....
    }
    finally
    {
        someclass.Dispose();
    }
}

Otros consejos

The best is to create and delete it in one place. Like

using(var context = factory.CreateContext())
{
    // use your context
}

Can you not add a Dispose method on your abstract class. Maybe implement IDisposable.

Let the provider of the calling code use a using block.

So on the abstract class:

 public void Dispose()
 {
        if (_context != null)
        {
            _context.Dispose();
        }
 }

Calling code:

using(var db = new Test())
{
... 
}

The downside of this is you're relying on the calling code to manage the disposable of your context.

It's not absolutely necessary to dispose of the context, although it is probably desired. The garbage collector will eventually dispose it, and entity framework uses ado.net connection pooling so it will not really be holding a connection.

It's not the utmost efficient to do this, but it won't cause any leaks, unless you have leaks in your class itself. Whenever your Test class goes out of scope, then the context will eventually be disposed by the gc.

You would really only be concerned if you were creating a lot of these objects over and over again, which is probably not the case.

I'm not suggesting this is a good practice, but if this is a bug fix of some type, or a temporary thing.. it's probably not necessary.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top