LINQ and the AutoCAD .NET API (Part 2)

Staying within a transaction’s scope

This is the second in a series of posts on LINQ an the AutoCAD .NET API. Here's a complete list of posts in this series.

Introduction

In the previous post we noticed that our first strategy of returning LayerTableRecord objects was problematic. Today I want to go into more detail about that.

The problem was already addressed in this post at Through the Interface. Let's look at the example from Through the Interface:

public static IEnumerable<Line> GetAllLines(Database db)
{
  using (var docLock = Application.DocumentManager.MdiActiveDocument.LockDocument())
  {
    using (var tr = db.TransactionManager.StartTransaction())
    {
      var bt = tr.GetObject(db.BlockTableId, OpenMode.ForRead) as BlockTable;
      var btr = tr.GetObject(bt[BlockTableRecord.ModelSpace], OpenMode.ForRead) as BlockTableRecord;

      foreach (var obj in btr)
      {
        var line = tr.GetObject(obj, OpenMode.ForRead) as Line;

        if (line != null)
        {
          yield return line;
        }
      }
    }
  }
}

In its structure, this code is the same as our first version of the GetLayers() code in Part 1. And Kean (the author of Through the Interface) comes to the same conclusion, that it is in general not a good idea, but it should be OK to use the method in a lazy context, like we do in listing 4 in part 1. Kean's comment on the code above:

In this case, the code is returning an IEnumerable of Lines. I strongly recommend not doing this: the Lines have been opened by the current transaction, and so will become invalid as soon as the transaction completes. Because we’re actually getting the results lazily – and therefore the calling function is getting the results one at a time, and so any function you call right afterwards will still have the transaction in scope – then this should be OK in this specific context.

So it's not a good idea to write code like the first version of our GetLayers() method, even though it might work in some cases. Especially in listing 5 in part 1 we walk on thin ice.

Deep dive

To find out more about the root cause of the problem, we have to tweak our code a bit. Let's add some debugging information to our first version of GetLayers() that helps us to examine the enumerator in more detail:

public static class LayerHelper
{
  private class Wrapper : IDisposable
  {
    private readonly Transaction tr;

    public Wrapper(Transaction tr)
    {
      this.tr = tr;
    }

    public void Dispose()
    {
      tr.Dispose();
      Debug.WriteLine("Transaction disposed of! Do NOT use any remaining objects from this transaction!");
    }
  }

  public static IEnumerable<LayerTableRecord> GetLayers()
  {
    // Get the current document and database
    Document acDoc = Application.DocumentManager.MdiActiveDocument;
    Database acCurDb = acDoc.Database;

    // Start a transaction
    Transaction acTrans = acCurDb.TransactionManager.StartTransaction();

    using (var trWrapper = new Wrapper(acTrans))
    {
      // Open the Layer table for read
      LayerTable acLyrTbl;
      acLyrTbl = acTrans.GetObject(acCurDb.LayerTableId, OpenMode.ForRead) as LayerTable;

      foreach (ObjectId acObjId in acLyrTbl)
      {
        var layer = acTrans.GetObject(acObjId, OpenMode.ForRead) as LayerTableRecord;
        Debug.WriteLine("Yielding layer");
        yield return layer;
      }
    }
  }
}

So what happens here? Two things are new: we created a new class called Wrapper, whose purpose is to intercept the call to the transaction's Dispose() method. So when Dispose() is called, it's first called on the Wrapper, then the Wrapper calls Dispose() on the transaction and writes "Transaction disposed of! [...]" to the console. So it's just an interceptor to properly output when Dispose() has been called on the transaction. The other new thing is that we write "Yielding layer" to the console whenever GetLayers() returns a LayerTableRecord object.

Now let's execute this code. Let's assume that we have three layers in our drawing: "0", "1" and "2". We use the client code from listing 4 in part 1 with two modifications: we print out "Accessing layer" before we access a layer's name and we use Debug.WriteLine() to output the final message. So here it is:

[CommandMethod("DisplayLayerNames5")]
public static void DisplayLayerNames5()
{
  var layerNames = new StringBuilder();

  foreach (var layer in LayerHelper.GetLayers())
  {
    Debug.WriteLine("Accessing layer");
    layerNames.Append($"\n{layer.Name}");
  }

  Debug.WriteLine($"The layers in this drawing are: {layerNames}");
}

And here's the output:

Yielding layer
Accessing layer
Yielding layer
Accessing layer
Yielding layer
Accessing layer
Transaction disposed of! Do NOT use any remaining objects from this transaction!
The layers in this drawing are:
0
1
2

I would say the result is actually what we expected: the enumerator yields a layer, then the name is appended to the StringBuilder. The enumerator yields another layer, and the name is appended... (and so on and so forth (lines 1-6 in the output)). After that, the transaction is being disposed of (line 7 in the output) and we print the final result (lines 8-11 in the output). Everything is fine here, because we access all layer objects before we dispose of the transaction.

Alright. Now let's look at the client code from listing 5 in part 1, the one that uses ToList():

[CommandMethod("DisplayLayerNames6")]
public static void DisplayLayerNames6()
{
  var layerNames = new StringBuilder();
  var list = LayerHelper.GetLayers().ToList();

  foreach (var layer in list)
  {
    Debug.WriteLine("Accessing layer");
    layerNames.Append($"\n{layer.Name}");
  }

  Debug.WriteLine($"The layers in this drawing are: {layerNames}");
}

What output do we get here?

Yielding layer
Yielding layer
Yielding layer
Transaction disposed of! Do NOT use any remaining objects from this transaction!
Accessing layer
Accessing layer
Accessing layer
The layers in this drawing are:
0
1
2

Ouch! Not really what we wanted, and finally the demonstartion of our problem: in lines 5-7 of the output we can see that we access the layer objects, even though the transcation has already been disposed of!

Why is that? The "problem" is the call to ToList(): ToList() as well as ToArray() materializes an IEnumerable<T>. That means that, instead of returning one layer object after another to the caller (like in listing 3), the layers are put into a list one after another, and the whole list is returned to the caller. So putting the layers into the list is what produces the lines 1-3 in the output. And because the enumerator has returned all objects when they were put into the list, we are done in GetLayers() and finally dispose of the transaction (line 4 in the output). But: we did not yet access the layer names and the transaction is already gone! That's the catch, finally.

Conclusion

So, the two main conclusions from this experiment are:

1. It is a bad idea to access AutoCAD database objects after the transaction they stem from has been disposed of
2. Therefore: returning and IEnumerable of AutoCAD database objects outside of the scope of a transaction will most probably lead to troubles and should be avoided

This leads to one simple but important rule:

Hm. Sounds reasonable, but actually this is not a really big insight, reading the .NET Developer's Guide will tell us that.

This is true, but I would say that it was important for our understanding to examine the interplay of IEnumerable and AutoCAD transactions.

Implementation

Alright. So, just to recap, this is our safe, but not so nice version of GetLayers() from Part 1:

public static class LayerHelper
{
  public static IEnumerable<LayerTableRecord> GetLayers(Database db, Transaction tr)
  {
    var table = (LayerTable)tr.GetObject(db.LayerTableId, OpenMode.ForRead);

    foreach (var id in table)
    {
      yield return (LayerTableRecord)tr.GetObject(id, OpenMode.ForRead);
    }
  }
}

And here's the client code:

[CommandMethod("DisplayLayerNames7")]
public static void DisplayLayerNames7()
{
  var db = Application.DocumentManager.MdiActiveDocument.Database;

  using (var tr = db.TransactionManager.StartTransaction())
  {
    var layerNames = new StringBuilder();
    var list = LayerHelper.GetLayers(db, tr).ToList();

    // No problem here
    foreach (var layer in list)
    {
      layerNames.Append($"\n{layer.Name}");
    }

    Application.ShowAlertDialog($"The layers in this drawing are: {layerNames}");
  }
}

We still want to get rid of the boiler plate code, and explicitly starting a transaction is boiler plate code. So it would be great to pack it somehow into our LayerHelper again. And now we know how to do it correctly: The transaction has to be the first thing that is created, then we interact with the database objects and the last thing we do is that we dispose of the transaction.

OK, so hiding the transaction and making it the first thing that is created: sounds if a constructor is the thing we need. So let's change our LayerHelper from a static class to a class we can instanciate and let's move the transaction parameter from the GetLayers() method to the constructor:

public class LayerHelper
{
  private readonly Transaction tr;

  public LayerHelper(Transaction tr)
  {
    this.tr = tr;
  }

  public IEnumerable<LayerTableRecord> GetLayers(Database db)
  {
    var table = (LayerTable)tr.GetObject(db.LayerTableId, OpenMode.ForRead);

    foreach (ObjectId id in table)
    {
      yield return (LayerTableRecord)tr.GetObject(id, OpenMode.ForRead);
    }
  }
}

OK, doesn't look too bad. What about the Database parameter? Actually we only pass it in to get to the LayerTableId. So the database parameter can be moved to the constructor as well:

public class LayerHelper
{
  private readonly Database db;
  private readonly Transaction tr;

  public LayerHelper(Database db, Transaction tr)
  {
    this.db = db;
    this.tr = tr;
  }

  public IEnumerable<LayerTableRecord> GetLayers()
  {
    var table = (LayerTable)tr.GetObject(db.LayerTableId, OpenMode.ForRead);

    foreach (var id in table)
    {
      yield return (LayerTableRecord)tr.GetObject(id, OpenMode.ForRead);
    }
  }
}

Looks even better! And now we can "wrap" the transaction: We add a parameterless constructor that starts the transaction using the database of the current document, and we implement IDisposable to dispose of the transaction, when the LayerHelper is disposed of:

public class LayerHelper : IDisposable
{
  private readonly Database db;
  private readonly Transaction tr;

  public LayerHelper()
    : this(Application.DocumentManager.MdiActiveDocument.Database,
    	   db.TransactionManager.StartTransaction())
  {
  }

  public LayerHelper(Database db, Transaction tr)
  {
    this.db = db;
    this.tr = tr;
  }

  public void Dispose()
  {
    if (tr != null && !tr.IsDisposed)
    {
      tr.Dispose();
    }
  }

  public IEnumerable<LayerTableRecord> GetLayers()
  {
    var table = (LayerTable)tr.GetObject(db.LayerTableId, OpenMode.ForRead);

    foreach (ObjectId id in table)
    {
      yield return (LayerTableRecord)tr.GetObject(id, OpenMode.ForRead);
    }
  }
}

If we now look at our client code, we're back at a cleaner version. But it's not only clean, now it's clean and safe, so using ToList()is no problem:

[CommandMethod("DisplayLayerNames8")]
public static void DisplayLayerNames8()
{
  using (var helper = new LayerHelper())
  {
    var layerNames = new StringBuilder();
    var list = helper.GetLayers().ToList();

    // No problem here
    foreach (var layer in list)
    {
      layerNames.Append($"\n{layer.Name}");
    }

    Application.ShowAlertDialog($"The layers in this drawing are: {layerNames}");
  }
}

This client code is actually what we wanted achieve in Part 1. It looks good, works, and is safe!

What did we win?

So what did we win in comparison to the code we started from in Part 1?

• Probably the most important win is that our client code is really just client code and that we reduced the amount of boilerplate code to a minimum. As Add-In developers we actually have to know only two things:

1. That there's a LayerHelper class that we have to instanciate
2. That the LayerHelper has to be disposed of when we're done and that a using block is the nicest way to do that

• Another good thing is that the LayerHelper hides all database semantics. From the client code perspective, there's no database involved, just objects.
• We can perform LINQ queries directly on the return value of GetLayers(), no need to cast or convert in the client code.
• We can use as many LINQ queries as we want to, provided that we stay inside the LayerHelper's using context. ToList() and ToArray() can be used easily.

What's next?

OK, one question: what about the using statement? You were so picky about getting rid of the transaction because we were creating it each time in the client code. But now we create a LayerHelper each time. What did we win there?

That's true. We can't get around the dispose pattern, we need an outermost using-block that "wraps" our database code. But if we can find a way to make the LayerHelper more general and make this more general class our entry point into the non-database-but-purely-object-based world, we would have a big win. Look at this code for example:

[CommandMethod("MyCommand")]
public static void MyCommand()
{
  using (var helper = new GeneralHelper())
  {
    var layers = helper.GetLayers();
    var blocks = helper.GetBlocks();
    var viewports = helper.GetViewports();
    // And so on
  }
}

The helper is the object we get all the goodness form. And as we have our outermost using block, we are safe to use the database objects and LINQ queries in whatever form.

So the next thing is to see if we can carve out a general pattern from the GetLayers() code that applies to other tables in the drawing database as well (like blocks or viewports). More on that in the next post.