Hey TT
"Well, I'm sure everyone would agree that using Open() and Close() (or Dispose) that way is going to become a problem if the code fails, so I guess it's about how one approaches the problem, and whether they plan for failure or always think about what'll happen in the event of a failure, or maybe they're the type that blissfully presumes that their code can't fail, and so they don't have to worry about what happens (e.g., what state the drawing is left in) if it does."
I don't agree, the using statement's Dispose is automatically called even when an exception is thrown. Also, there is a Cancel() method on an Opened entity which undoes the changes. This is the same function that a a StartOpenCloseTransaction.Abort() calls.
Hey Fenton. I think you've misunderstood me. That is the whole point of my comment - that DBObject.Dispose() calls AcDbObject::close(), which is
incorrect if an exception was thrown before the call to Dispose() is made by using().
That is precisely why Transaction.Dispose() calls Abort() rather than Commit(), if Commit() was not previously called.
Here is your 'nicer' code from that post:
Now my preferred way, Open/Close – see how much nicer it is?
public void OpenClose()
{
Database db = HostApplicationServices.WorkingDatabase;
ObjectId msId = SymbolUtilityServices.GetBlockModelSpaceId(db);
using (BlockTableRecord btr = (BlockTableRecord)msId.Open(OpenMode.ForRead))
{
foreach (ObjectId id in btr)
{
using (Entity ent = (Entity)id.Open(OpenMode.ForWrite))
{
// do something
}
}
}
}
Now I have shown the differences between the two styles...
You really haven't shown
all the differences between the two styles.
There was one important difference, which is what happens when an exception terminates the code within the inner using() block. When using a transaction, all changes made to any object subsequent to starting the transaction will be rolled back. In your example above, all changes made to all objects prior to the point where the exception terminates the code, will be left intact, and will
not be rolled back, and that is because DBObject.Dispose() calls AcDbObject::close(), which commits changes. So, all I was trying to point out was that your example above is functionally not equivalent to any of the others, all of which will roll back changes to all objects if the code is stopped by an exception.
So, if you want to compare apples with apples, your 'nicer' version must roll back changes to all entities processed by the foreach() loop when an exception is raised, which it doesn't do, making it functionally-incomplete, relative to what the methods that demonstrate the other 3 styles do.
And, for the much simpler case of using ObjectId.Open() to modify a single entity, here is what is actually required:
using (Entity ent = (Entity)id.Open(OpenMode.ForWrite))
{
try
{
// do something
}
catch
{
ent.Cancel();
ent = null; // supress call to Dispose() and AcDbObject::close()
throw;
}
}
And I think that snippet serves to show that using ObjectId.Open() to modify objects is not nearly as simple as it might at first seem, especially to someone who is used to thinking in terms of the Acad::ErrorStatus returned by most native API calls
. It really does take a lot of mental 'retooling' to think about conveying failure using exceptions as it's done in managed code.
So if your response to this is 'well then you should use an OpenCloseTransaction', that's all fine and dandy except that someone neglected to think about the need to get the current/active OpenCloseTransaction without having to pass it around like a jug of moonshine.
