3

This is a fairly fine point, and I expect the answer is "it's not a hot idea to begin with" - that said, it has a points that I'm interested in regardless, if someone is kind enough to indulge.

Model Code:

public partial class MyEntities : ObjectContext
{
    // the idea is if the object is in a using block, this always gets called?
    protected override void Dispose(bool disposing)
    {
        this.SaveChanges();
        base.Dispose(disposing);
    }
}

Client Code:

using(var model = new MyEntities())
{
   // do something

   // no worry about calling model.SaveChanges()
}

The issues I'm uncertain about are:

  1. Is Dispose the right place to do this because I was thinking "Finalize" for some reason - I always get confused on C# destruction.

  2. In the case an exception is thrown in the client code, ordinarily SaveChanges would be skipped and that's good, but if this works how I think, it'll always call it. Should I use try with an empty catch?

    public partial class MyEntities : ObjectContext
    {
        protected override void Dispose(bool disposing)
        {
            try
            {
               this.SaveChanges();
            }
            catch {}
            base.Dispose(disposing);
        }
    }
    
Bo Persson
  • 90,663
  • 31
  • 146
  • 203
Aaron Anodide
  • 16,906
  • 15
  • 62
  • 121
  • -1 Dispose is for cleaning up resources. Calling SaveChanges there is very bad. – Tomas Voracek Sep 12 '11 at 20:45
  • is it's invocation tied to the scope of the containing stack frame or tied to the garbage collector? that's what I think I was really wondering now that you made me think more. – Aaron Anodide Sep 12 '11 at 20:47
  • You can't let anybody see this code. However, the neckshot is that you'll call SaveChanges() even when the code throws an exception. That's a great way to mess up a dbase. – Hans Passant Sep 12 '11 at 22:31
  • possible duplicate of [Saving a Class to disk on dispose: Does my code have bugs?](http://stackoverflow.com/questions/3832911/saving-a-class-to-disk-on-dispose-does-my-code-have-bugs) – Brian Sep 13 '11 at 11:40

2 Answers2

13

Don't do this. It's a bad idea.

The purpose of "Dispose" is to politely dispose of an unmanaged resource early so that other processes can use it. "Dispose" should not have semantics -- it should not change the state of your program or be required in some way. It should only do precisely what it says it does: dispose of a resource.

Should you do it in the finalizer? Absolutely not. That's even worse. The finalizer might not run at all, the finalizer runs on another thread, the finalizer can be called even if the object wasn't properly initialized, and so on. Writing a finalizer is almost never the right thing to do, and if you do write a finalizer it should only dispose of a resource. Do not do anything fancy in a finalizer; you will almost certainly write a dangerously incorrect and brittle program if you do.

The right principle to cleave to here is: if a call is required for semantic reasons then force the user to put the call in the code. If they forget to do it, they'll find out in testing. Let the user decide whether it is the right thing to do to put the call in a finally block or not. Don't make that decision for them; you might decide wrong.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • Similar question and unfortunate accepted answer that does not point out that this should not be done in `IDisposable.Dispose` [here](http://stackoverflow.com/questions/7295198/when-is-destructor-called-in-a-wcf-service/7295325#comment-8790533). – jason Sep 13 '11 at 13:39
  • @Greg, i started out a C++ programmer from '96 to '01... C# ever since.. I've been profiled correctly – Aaron Anodide Sep 13 '11 at 17:10
0
  1. Dispose is where you would do this, if you were to do it at all.

  2. This is one of the reasons you should not do it.

Andrew Barber
  • 39,603
  • 20
  • 94
  • 123
  • i suppose a programming language could conceivably have a "success block" control structure – Aaron Anodide Sep 12 '11 at 20:42
  • That's what OO and architecture is for. Create a class that has an extensible "success block" but is perhaps defined with some base functionality in your abstract class. – Jaxidian Jan 20 '13 at 19:11