Author Topic: Wrapping command actions  (Read 8831 times)

0 Members and 1 Guest are viewing this topic.

zoltan

  • Guest
Wrapping command actions
« on: May 08, 2012, 03:22:02 PM »
In an effort to separate a command definition from it's implementation, I came up with up with some wrapper methods to wrap delegates with Transactions and Document Locks.

Code - C#: [Select]
  1. public static class ErrorHandler
  2. {
  3.         public static void Execute(Action action)
  4.         {
  5.             try
  6.             {
  7.                 action();
  8.             }
  9.             catch ()
  10.             {
  11.                 // Handle your screwy exceptions here!
  12.             }
  13.         }
  14.  
  15.         public static Action WrapWithTransaction(Action<Transaction> action)
  16.         {
  17.             return delegate()
  18.             {
  19.                 Document acadDocument = Application.DocumentManager.MdiActiveDocument;
  20.  
  21.                 using (Transaction trans = acadDocument.TransactionManager.StartTransaction())
  22.                 {
  23.                     action(trans);
  24.  
  25.                     trans.Commit();
  26.                 }
  27.             };
  28.         }
  29.  
  30.         public static Action WrapWithDocumentLock(Action action, string globalCommandName, string localCommandName, bool promptIfFail)
  31.         {
  32.             return delegate ()
  33.             {
  34.                 Document acadDocument = Application.DocumentManager.MdiActiveDocument;
  35.  
  36.                 using (acadDocument.LockDocument(DocumentLockMode.Write, globalCommandName, localCommandName, promptIfFail))
  37.                 {
  38.                     action();
  39.                 }
  40.             };
  41.         }
  42. }
  43.  
  44.  

In your command class, create an implementation method that does the work of your command and wrap it up.

Code - C#: [Select]
  1.     public class MyCommands
  2.     {
  3.         [CommandMethod("GroupName", "MyCommandGlobalName", "MyCommandLocalizedName", CommandFlags.Modal)]
  4.         public void MyCommand()
  5.         {
  6.             ErrorHandler.Execute
  7.             (
  8.                 ErrorHandler.WrapWithDocumentLock
  9.                 (
  10.                     ErrorHandler.WrapWithTransaction
  11.                     (
  12.                         MyCommandImpl
  13.                     ),
  14.                     "MyCommandGlobalName",
  15.                     "MyCommandLocalizedName",
  16.                     false
  17.                 )
  18.             );
  19.         }
  20.  
  21.         private static void MyCommandImpl(Transaction trans)
  22.         {
  23.            // Command does the work here.
  24.         }
  25.     }
  26.  


So, what does everybody think?
« Last Edit: May 08, 2012, 03:38:48 PM by zoltan »

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #1 on: May 08, 2012, 08:07:22 PM »
In an effort to separate a command definition from it's implementation, I came up with up with some wrapper methods to wrap delegates with Transactions and Document Locks.


From what I can see, your abstraction requires one
to write more code to implement a command.

E.g., how is it any different or simpler than this:

Code - C#: [Select]
  1.  
  2.    public static class MyCommands
  3.    {
  4.         [CommandMethod("MYCOMMAND")]
  5.         public static void MyCommandMethod()
  6.         {
  7.              Document doc = Application.DocumentManager.MdiActiveDocument;
  8.              using( doc.LockDocument() )
  9.              using( Transaction tr = doc.TransactionManager.StartTransaction() )
  10.              {
  11.                   try
  12.                   {
  13.                         MyCommandWorker();
  14.                         tr.Commit();
  15.                   }
  16.                   catch( System.Exception ex )
  17.                   {
  18.                         OnError( ex );  
  19.                   }
  20.              }
  21.         }
  22.  
  23.         public static void MyCommandWorker()
  24.         {
  25.              // do work here;
  26.         }
  27.  
  28.         public static void OnError( Exception ex )
  29.         {
  30.              // screwy error handling here
  31.         }
  32.    }
  33.  
  34.  

Quote

Code - C#: [Select]
  1. public static class ErrorHandler
  2. {
  3.         public static void Execute(Action action)
  4.         {
  5.             try
  6.             {
  7.                 action();
  8.             }
  9.             catch ()
  10.             {
  11.                 // Handle your screwy exceptions here!
  12.             }
  13.         }
  14.  
  15.         public static Action WrapWithTransaction(Action<Transaction> action)
  16.         {
  17.             return delegate()
  18.             {
  19.                 Document acadDocument = Application.DocumentManager.MdiActiveDocument;
  20.  
  21.                 using (Transaction trans = acadDocument.TransactionManager.StartTransaction())
  22.                 {
  23.                     action(trans);
  24.  
  25.                     trans.Commit();
  26.                 }
  27.             };
  28.         }
  29.  
  30.         public static Action WrapWithDocumentLock(Action action, string globalCommandName, string localCommandName, bool promptIfFail)
  31.         {
  32.             return delegate ()
  33.             {
  34.                 Document acadDocument = Application.DocumentManager.MdiActiveDocument;
  35.  
  36.                 using (acadDocument.LockDocument(DocumentLockMode.Write, globalCommandName, localCommandName, promptIfFail))
  37.                 {
  38.                     action();
  39.                 }
  40.             };
  41.         }
  42. }
  43.  
  44.  

In your command class, create an implementation method that does the work of your command and wrap it up.

Code - C#: [Select]
  1.     public class MyCommands
  2.     {
  3.         [CommandMethod("GroupName", "MyCommandGlobalName", "MyCommandLocalizedName", CommandFlags.Modal)]
  4.         public void MyCommand()
  5.         {
  6.             ErrorHandler.Execute
  7.             (
  8.                 ErrorHandler.WrapWithDocumentLock
  9.                 (
  10.                     ErrorHandler.WrapWithTransaction
  11.                     (
  12.                         MyCommandImpl
  13.                     ),
  14.                     "MyCommandGlobalName",
  15.                     "MyCommandLocalizedName",
  16.                     false
  17.                 )
  18.             );
  19.         }
  20.  
  21.         private static void MyCommandImpl(Transaction trans)
  22.         {
  23.            // Command does the work here.
  24.         }
  25.     }
  26.  

So, what does everybody think?

On a side note, does anyone know why code=csharp is indenting so heavily?
« Last Edit: May 08, 2012, 08:11:24 PM by TheMaster »

Kerry

  • Mesozoic relic
  • Seagull
  • Posts: 11654
  • class keyThumper<T>:ILazy<T>
Re: Wrapping command actions
« Reply #2 on: May 08, 2012, 08:37:30 PM »

I prefer the 'conventional' way of doing this.
I feel it's more transparent.
.. and it is easier to refactor when necessary.


but that's just me :)
kdub, kdub_nz in other timelines.
Perfection is not optional.
Everything will work just as you expect it to, unless your expectations are incorrect.
Discipline: None at all.

zoltan

  • Guest
Re: Wrapping command actions
« Reply #3 on: May 08, 2012, 09:41:56 PM »
I guess my example did not really give the reason why I chose to break it out this way.

Ideally, the MyCommandImpl static method would be in another class, one that contains all of the implementation for what that command does, which is often times made up of multiple methods.

I discovered that when I kept the implementation for the command in the command class, the command class became very large and hard to maintain.

It would start looking like this:
Code - C#: [Select]
  1. [assembly: CommandClass(typeof(MyCommands))]
  2. public class MyCommands
  3. {
  4.   [CommandMethod("MyCommand")]
  5.   public void MyCommand()
  6.   {
  7.     Foo();
  8.   }
  9.  
  10.   private void Foo()
  11.   {
  12.     Bar();
  13.   }
  14.  
  15.   private void Bar()
  16.   {
  17.     // Do some stuff
  18.   }
  19.  
  20.   [CommandMethod("MyOtherCommand")]
  21.   public void MyOtherCommand()
  22.   {
  23.     Baz();
  24.   }
  25.  
  26.   private void Baz()
  27.   {
  28.     // Do some other stuff
  29.   }
  30. }
  31.  
  32.  

It would start to get very large and hard to find the commands.  So then I started putting all of my command methods in separate classes to keep them organized.  Then, I decided that my command methods were all scattered everywhere and still hard to find.  Also, the CommandClass attribute can only be on one class and it optimized the loading of the commands since it tells AutoCAD that all of the command methods are all in one class.

Quote
An application may designate one, and only one, type as its command class. AutoCAD looks for an application's command methods on the type that bears this attribute.

Even though the documentation says that all of the CommandMethod attributes must be in a single class that is attributed with the CommandClass attribute:

Quote
Command methods may be defined only in a class that is marked with the CommandClass attribute.
I have found that this is not true.

So, keeping all of your command definitions in one place makes sense, because you know where to find them.  Keeping all of the commands' implementations separate also makes sense because it keeps them encapsulated.

So this is what I do now:

Code - C#: [Select]
  1. [assembly: CommandClass(typeof(MyCommands))]
  2. public class CommandClass
  3. {
  4.   [CommandMethod("MyCommand")]
  5.   public void MyCommand()
  6.   {
  7.     MyCommand.CommandImpl();
  8.   }
  9.  
  10.   [CommandMethod("MyOtherCommand")]
  11.   public void MyOtherCommand()
  12.   {
  13.     MyOtherCommand.CommandImpl();
  14.   }
  15. }
  16.  

And then the command implementations are in their own classes.

Code - C#: [Select]
  1.  
  2. internal class MyCommand
  3. {
  4.   public static void CommandIml()
  5.   {
  6.     Foo();
  7.   }
  8.  
  9.   private static void Foo()
  10.   {
  11.     Bar();
  12.   }
  13.  
  14.   private static void Bar()
  15.   {
  16.     // Do some stuff
  17.   }
  18. }
  19.  
  20. internal class MyOtherCommand
  21. {
  22.   public static void CommandImpl()
  23.   {
  24.     Baz();
  25.   }
  26.  
  27.   private static void Baz()
  28.   {
  29.     // Do some other stuff
  30.   }
  31. }
  32.  


The purpose of the wrapper method is to allow for reuse.

Consider the following traditional example:
Code - C#: [Select]
  1. public void MyCommand()
  2. {
  3.   try
  4.   {
  5.     // do some stuff
  6.   }
  7.   catch (Exception ex)
  8.   {
  9.     // Show a message to the user.
  10.  
  11.     // Log the exception.
  12.  
  13.     // Do some cleanup.
  14.   }
  15. }
  16.  
  17. public void MyOtherCommand()
  18. {
  19.   try
  20.   {
  21.     // do some other stuff
  22.   }
  23.   catch (Exception ex)
  24.   {
  25.     // Show a message to the user.
  26.  
  27.     // Log the exception.
  28.  
  29.     // Do some cleanup.
  30.   }
  31. }
  32.  
  33. public void MyThirdCommand()
  34. {
  35.   try
  36.   {
  37.     // do even more stuff
  38.   }
  39.   catch (Exception ex)
  40.   {
  41.     // Show a message to the user.
  42.  
  43.     // Log the exception.
  44.  
  45.     // Do some cleanup.
  46.   }
  47. }
  48.  

We are repeating a lot of logic in the error handling that can be reused.

Here is how:
Code - C#: [Select]
  1. public void MyCommand()
  2. {
  3.   ErrorHandler.Execute(MyCommand.CommandImpl);
  4. }
  5.  
  6. public void MyOtherCommand()
  7. {
  8.   ErrorHandler.Execute(MyOtherCommand.CommandImpl);
  9. }
  10.  
  11. public void MyThirdCommand()
  12. {
  13.   ErrorHandler.Execute(MyThirdCommand.CommandImpl);
  14. }
  15.  

All of the commands use the same error handler and changing that logic will change all of the commands globally.

Not all of the commands will use all of the wrappers, and the wrappers can be used anywhere in code.  For example, if a command wants to lock the document multiple times, it would use the WrapWithDocumentLock wrapper multiple times in the body of the command implementation.  The code only has to be written once and reused many times.  The wrappers allow for the functionality of the commands to be more granular.

While it may look like a lot more work up front for a single command, I think it is more maintainable and reusable, and better organized as the application becomes large and contains many commands.


zoltan

  • Guest
Re: Wrapping command actions
« Reply #4 on: May 08, 2012, 09:43:52 PM »
Quote
On a side note, does anyone know why code=csharp is indenting so heavily?

It doesn't.  I just copied the code out of Visual Studio, and it came over that way.

Kerry

  • Mesozoic relic
  • Seagull
  • Posts: 11654
  • class keyThumper<T>:ILazy<T>
Re: Wrapping command actions
« Reply #5 on: May 08, 2012, 10:05:00 PM »
< ..>.

I discovered that when I kept the implementation for the command in the command class, the command class became very large and hard to maintain.

< .. >

Have a look at the partial keyword

.. can be used with a class and a method.

great for 'hiding' the boring stuff :)



ps: your real name will probably come to me tonight some time
kdub, kdub_nz in other timelines.
Perfection is not optional.
Everything will work just as you expect it to, unless your expectations are incorrect.
Discipline: None at all.

zoltan

  • Guest
Re: Wrapping command actions
« Reply #6 on: May 08, 2012, 11:21:38 PM »
Have a look at the partial keyword

Sure, you can split a class into as many CS files as you want, but it is still one, big class... with lots of members.

And it does not help you with reuse, or the ideas of encapsulation and separation of concerns.

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #7 on: May 09, 2012, 01:37:29 AM »

Quote
An application may designate one, and only one, type as its command class. AutoCAD looks for an application's command methods on the type that bears this attribute.

Even though the documentation says that all of the CommandMethod attributes must be in a single class that is attributed with the CommandClass attribute:

Quote
Command methods may be defined only in a class that is marked with the CommandClass attribute.
I have found that this is not true.


Yes, that's right. You can have any number of CommandClass attributes in an assembly, each naming a different class. If there's at least one CommandClass attribute, then the runtime will only search the classes named by CommandClass attributes for commands.

If Autodesk's documentation is what you are quoting above, then it's wrong.

Quote

The purpose of the wrapper method is to allow for reuse.
...

All of the commands use the same error handler and changing that logic will change all of the commands globally.

Code - C#: [Select]
  1.  
  2. public void MyCommand()
  3. {
  4.   try
  5.   {
  6.     // do some stuff
  7.   }
  8.   catch (Exception ex)
  9.   {
  10.     // Show a message to the user.
  11.  
  12.     // Log the exception.
  13.  
  14.     // Do some cleanup.  <<<<<<<  ???
  15.   }
  16. }
  17.  
  18.  


If I need to do some cleanup (whether an error occurs or not) in a command method, I don't use try/catch, I use try/finally.

If you're doing some kind of logging of exceptions, that could be done just as easily by calling the API that does those things from a catch block in each command method. The advantage of that is that if other specialized exception handling must also be done for some, but not all commands, it is easier to do that and still reuse common exception handling code as well.

The problem with encapsulating try/catch is that it makes it more difficult to have specialized exception handling code that may be needed for some, but not all commands, and still have available the things that are encapsulated into a common try/catch block in a called API.

The other methods you posted that merely encapsulate the basic tasks of document locking and starting/committing a transaction require the same amount of code to use than to not use. That's because you have separate wrappers for document locking and transactions, which if you ask me, is unnecessary overkill.

If you wanted to use an action to encapsulate the starting and committing of a transaction and document locking, that's a legitimate reason for using an action IMO, but you don't have to call methods that wrap actions inside other actions to achieve it.

Here's an example (untested) that promotes the same type of separation used by AutoCAD's managed API:

Code - C#: [Select]
  1.  
  2. public static class DocumentExtensionMethods
  3. {
  4.  
  5.   // Locks the active document, starts a transaction, and
  6.   // calls a delegate that takes a Database as its only
  7.   // parameter. This design allows the called delegate to
  8.   // have no dependence on AcMgd.dll (the AutoCAD application
  9.   // layer), which permits the delegate to be used in RealDwg
  10.   // applications, outboard databases opened via ReadDwgFile(),
  11.   // and in the AutoCAD core console in addition to being used
  12.   // on databases open in the AutoCAD editor.
  13.   //
  14.   // This extension method also demonstrates a more compelling
  15.   // and useful purpose to and reason for maintaining a clear
  16.   // separation between implementation and UI (command methods
  17.   // are actually a just a non-graphical type of UI).
  18.  
  19.   public static void ExecuteInTransaction( this DocumentCollection docs,
  20.                                               Action<Database> action )
  21.   {
  22.     Document doc = docs.MdiActiveDocument;
  23.     using( doc.LockDocument() )
  24.     using( Transaction tr = doc.TransactionManager.StartTransaction() )
  25.     {
  26.       action( doc.Database );
  27.       tr.Commit();
  28.     }
  29.   }
  30.  
  31.   // This is like the above, but returns the result that
  32.   // is returned by the passed delegate, where T is the
  33.   // type of the result:
  34.  
  35.   public static T ExecuteInTransaction<T>( this DocumentCollection docs,
  36.                                               Func<Database, T> func )
  37.   {
  38.     Document doc = docs.MdiActiveDocument;
  39.     using( doc.LockDocument() )
  40.     using( Transaction tr = doc.TransactionManager.StartTransaction() )
  41.     {
  42.       T result = func( doc.Database );
  43.       tr.Commit();
  44.       return result;
  45.     }
  46.   }
  47.  
  48. }  
  49.  
  50. // Example usage of the above extension methods:
  51.  
  52. public static class MyCommands
  53. {
  54.  
  55.   // This uses a lambda expression:
  56.  
  57.   [CommandMethod("MYCOMMAND")]
  58.   public static void MyCommandMethod()
  59.   {
  60.     Application.DocumentManager.ExecuteInTransaction(
  61.  
  62.       delegate( Database db )
  63.       {
  64.         // Erase all circles in the current space, using
  65.         // helper APIs posted elsewhere on this site, cause
  66.         // I'm too damn lazy to write out the 'longhand' :p)
  67.  
  68.         var circles = db.CurrentSpace().GetObjects<Circle>();
  69.  
  70.         foreach( Circle circle in circles.UpgradeOpen() )
  71.         {
  72.           circle.Erase();
  73.         }
  74.       }
  75.  
  76.     );
  77.   }
  78.  
  79.   // This version uses a separate method which could be
  80.   // in another class, or in another assembly that has no
  81.   // dependence on AcMgd.dll (the AutoCAD application layer),
  82.   // and is included here for the purpose of demonstrating
  83.   // how to properly separate implementation from UI:
  84.  
  85.   [CommandMethod("MYCOMMAND2")]
  86.   public static void MyCommand2Method()
  87.   {
  88.     Application.DocumentManager.ExecuteInTransaction( EraseAllCircles );
  89.   }
  90.  
  91.   // This method is included in this class for illustration only,
  92.   // but could also be in another assembly that has no dependence
  93.   // on AcMgd.dll (the AutoCAD application layer). That allows the
  94.   // method to be used on outboard/external databases from within
  95.   // AutoCAD, or from a RealDwg application, or the core console in
  96.   // AutoCAD 2013. For that reason the method takes a Database as
  97.   // its parameter, rather than a Document:
  98.  
  99.   static void EraseAllCircles( Database db )
  100.   {
  101.     var circles = db.CurrentSpace().GetObjects<Circle>();
  102.  
  103.     foreach( Circle circle in circles.UpgradeOpen() )
  104.     {
  105.       circle.Erase();
  106.     }
  107.   }
  108.  
  109.  
  110.   // This example uses a called method that returns the sum of
  111.   // the area of all closed polylines in the modelspace of the
  112.   // active document, but that same method could also be used
  113.   // on any database in AutoCAD, a RealDwg host application, or
  114.   // the 2013 core console. The aspect of the 'separation' that
  115.   // isn't shown here is that methods like the following should
  116.   // reside in a separate assembly that has no dependence
  117.   // on AcMgd.dll, following the same pattern of separation used
  118.   // by AutoCAD's own managed API (where AcDbMgd.dll has no
  119.   // dependence on AcMgd.dll).
  120.  
  121.   [CommandMethod("SUMCURVEAREA")]
  122.   public static void SumCurveArea()
  123.   {
  124.     var docs = Application.DocumentManager;
  125.    
  126.     double area = docs.ExecuteInTransaction( SumCurveArea );
  127.    
  128.     docs.MdiActiveDocument.Editor.WriteMessage(
  129.       "Area of all closed polylines: {0} sq units", area  );
  130.   }
  131.  
  132.   // This method would be in another assembly that has no
  133.   // dependence on AcMgd.dll (the AutoCAD application layer):
  134.  
  135.   public static double SumCurveArea( Database db )
  136.   {
  137.     return db.CurrentSpace().GetObjects<Polyline>()
  138.       .Where( pline => pline.Closed )
  139.       .Sum( pline => pline.Area );
  140.   }
  141.  
  142. }
  143.  
  144.  

See http://www.theswamp.org/index.php?topic=41311.msg464457#msg464457
for the referenced extension methods.
Quote

All of the commands use the same error handler and changing that logic will change all of the commands globally.

Not all of the commands will use all of the wrappers, and the wrappers can be used anywhere in code.  For example, if a command wants to lock the document multiple times, it would use the WrapWithDocumentLock wrapper multiple times in the body of the command implementation. 


There's no need for a wrapper to lock a document. Locking a document requires a single line of code:

Code - C#: [Select]
  1.  
  2.     using( Application.DocumentManager.MdiActiveDocument.LockDocument() )
  3.     {
  4.          // do stuff with document locked.
  5.     }
  6.  
  7.  

There is also no point or purpose to using the overload that takes the lockmode and command names. That overload is intended for use from modeless contexts (e.g., when there is no registered command running).

Again, I don't see any point to having wrappers for locking documents and starting/committing transactions, as they require more code to use than to not use.

Quote

While it may look like a lot more work up front for a single command, I think it is more maintainable and reusable, and better organized as the application becomes large and contains many commands.


I disagree. For most commands that use transactions, there's little benefit to calling a wrapper that requires an action, and does nothing other than starts and commits the transaction before/after calling the action.

[5/09/12] edit: corrected bug pointed out by Alex
« Last Edit: May 09, 2012, 11:40:57 AM by TheMaster »

Alexander Rivilis

  • Bull Frog
  • Posts: 214
  • Programmer from Kyiv (Ukraine)
Re: Wrapping command actions
« Reply #8 on: May 09, 2012, 07:41:20 AM »
Code - C#: [Select]
  1.   public static T ExecuteInTransaction<T>( this DocumentCollection docs,
  2.                                               Func<Database, T> func )
  3.   {
  4.     Document doc = docs.MdiActiveDocument;
  5.     using( doc.LockDocument() )
  6.     using( Transaction tr = doc.TransactionManager.StartTransaction() )
  7.     {
  8.       return func( doc.Database );
  9.       tr.Commit();
  10.     }
  11.   }
  12.  
If I understand correctly, the code tr.Commit(); will never be executed ...


zoltan

  • Guest
Re: Wrapping command actions
« Reply #9 on: May 09, 2012, 08:27:57 AM »
The purpose of the wrapper functions is not to reduce the number of lines of code.  It is to make to code more granular and to abstract out what is common, encapsulating that common logic in it's own class and allowing it to change at run time.

Code - C#: [Select]
  1.  
  2.         abstract class AbstractWrapper
  3.         {
  4.             public abstract void ExcecuteAction(Action action);
  5.         }
  6.  
  7.         sealed class ConcreteWrapper1 : AbstractWrapper
  8.         {
  9.             public override void ExcecuteAction(Action action)
  10.             {
  11.                 CommonLogic1();
  12.  
  13.                 action();
  14.  
  15.                 CommonLogic2();
  16.             }
  17.         }
  18.  
  19.         sealed class ConcreteWrapper2 : AbstractWrapper
  20.         {
  21.             public override void ExcecuteAction(Action action)
  22.             {
  23.                 CommonLogic3();
  24.  
  25.                 action();
  26.  
  27.                 CommonLogic4();
  28.             }
  29.         }
  30.  

Code - C#: [Select]
  1.         public void ConcreteLogic()
  2.         {
  3.             AbstractWrapper myWrapper;
  4.  
  5.             if (SomeCondition())
  6.             {
  7.                 myWrapper = new ConcreteWrapper1();
  8.             }
  9.             else
  10.             {
  11.                 myWrapper = new ConcreteWrapper2();
  12.             }
  13.  
  14.             myWrapper.ExcecuteAction(() => { UniqueLogic(); });
  15.         }
  16.  


side node: Why is code=csarp really dumb about coloring key words?

zoltan

  • Guest
Re: Wrapping command actions
« Reply #10 on: May 09, 2012, 08:32:22 AM »
Code - C#: [Select]
  1.  public static void ExecuteInTransaction( this DocumentCollection docs,
  2.                                              Action<Database> action )
  3.  {
  4.    Document doc = docs.MdiActiveDocument;
  5.    using( doc.LockDocument() )
  6.    using( Transaction tr = doc.TransactionManager.StartTransaction() )
  7.    {
  8.      action( doc.Database );
  9.      tr.Commit();
  10.    }
  11.  }
  12.  

Why would you put the extension method on the DocumentCollection class instead of the Document class?

It's Alive!

  • Retired
  • Needs a day job
  • Posts: 8691
  • AKA Daniel
Re: Wrapping command actions
« Reply #11 on: May 09, 2012, 08:57:49 AM »
So this is what I do now:

Code - C#: [Select]
  1. [assembly: CommandClass(typeof(MyCommands))]
  2. public class CommandClass
  3. {
  4.   [CommandMethod("MyCommand")]
  5.   public void MyCommand()
  6.   {
  7.     MyCommand.CommandImpl();
  8.   }
  9.  
  10.   [CommandMethod("MyOtherCommand")]
  11.   public void MyOtherCommand()
  12.   {
  13.     MyOtherCommand.CommandImpl();
  14.   }
  15. }
  16.  

...

I like this the best..why add more layers of func  :wink:

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #12 on: May 09, 2012, 11:14:18 AM »
Code - C#: [Select]
  1.  public static void ExecuteInTransaction( this DocumentCollection docs,
  2.                                              Action<Database> action )
  3.  {
  4.    Document doc = docs.MdiActiveDocument;
  5.    using( doc.LockDocument() )
  6.    using( Transaction tr = doc.TransactionManager.StartTransaction() )
  7.    {
  8.      action( doc.Database );
  9.      tr.Commit();
  10.    }
  11.  }
  12.  

Why would you put the extension method on the DocumentCollection class instead of the Document class?


You could do either, I just wrote it that way because the Active document is implied and I don't foresee it being used on anything but the active document (and it made the usage example more succint), but yes it could target the Document as well.

Many AutoCAD APIs are document-centric, yet we almost always operate on the Active document exclusively, so that kind of API design can often lead to greater complexity, in spite of the fact that it is rarely, if ever needed.
« Last Edit: May 09, 2012, 11:22:45 AM by TheMaster »

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #13 on: May 09, 2012, 11:25:00 AM »
Good catch Alex  :laugh: I (obviously) didn't test it.

Code - C#: [Select]
  1.   public static T ExecuteInTransaction<T>( this DocumentCollection docs,
  2.                                               Func<Database, T> func )
  3.   {
  4.     Document doc = docs.MdiActiveDocument;
  5.     using( doc.LockDocument() )
  6.     using( Transaction tr = doc.TransactionManager.StartTransaction() )
  7.     {
  8.        T result = func( doc.Database );
  9.        tr.Commit();
  10.        return result;
  11.     }
  12.   }
  13.  

If I understand correctly, the code tr.Commit(); will never be executed ...
« Last Edit: May 09, 2012, 11:39:47 AM by TheMaster »

BlackBox

  • King Gator
  • Posts: 3770
Re: Wrapping command actions
« Reply #14 on: May 09, 2012, 11:34:20 AM »
side node: Why is code=csarp really dumb about coloring key words?

FWIW -

That is the result of many, many users asking for this forum enhancement, in addition to a lot of dedication and hard work by Se7en & others (but mostly thanks to Se7en):

http://www.theswamp.org/index.php?topic=39358.0


If, however, you'd rather not use the formatted code altogether, consider this post.

HTH
"How we think determines what we do, and what we do determines what we get."

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #15 on: May 09, 2012, 02:58:30 PM »
side node: Why is code=csarp really dumb about coloring key words?

FWIW -

That is the result of many, many users asking for this forum enhancement, in addition to a lot of dedication and hard work by Se7en & others (but mostly thanks to Se7en):

http://www.theswamp.org/index.php?topic=39358.0


If, however, you'd rather not use the formatted code altogether, consider this post.

HTH

Not to denigrate anyone's work, it's just that the code is hard to read using the predefined colors and font.  Most don't use the Courier font (I think I saw a survey once, and Lucida Sans Typewriter, which is what I've used for many years, is the #1 choice).

And, the background color in my Visual Studio editor is Indigo   :laugh:

BlackBox

  • King Gator
  • Posts: 3770
Re: Wrapping command actions
« Reply #16 on: May 09, 2012, 03:40:39 PM »
Not to denigrate anyone's work, it's just that the code is hard to read using the predefined colors and font.  Most don't use the Courier font (I think I saw a survey once, and Lucida Sans Typewriter, which is what I've used for many years, is the #1 choice).

Not at all, Tony, and I appreciate your saying that... I don't believe anyone was intentionally (or otherwise) denigrating others' work. I know Zoltan outside of the TheSwamp, so I was just trying to share what I knew, in a quick post while at my day job.  :wink:

Admittedly, there are things I dislike about the new code tags (i.e., the lack of scroll bar, forcing the code to post in full length, etc.). I only meant to point out that the current state of all things code tags is the result of a long experiment in feature enhancement.... more importantly, that the legacy [code ] tags are still available (manually).

And, the background color in my Visual Studio editor is Indigo   :laugh:

I'm considering changing my theme over to the Dark Side (from default).   :mrgreen:

/offtopic

Cheers! :beer:
"How we think determines what we do, and what we do determines what we get."

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #17 on: May 09, 2012, 03:52:11 PM »

The purpose of the wrapper functions is not to reduce the number of lines of code.  It is to make to code more granular and to abstract out what is common, encapsulating that common logic in it's own class and allowing it to change at run time.


Sorry to disagree, but in the case of your abstractions of try/catch, transactions, and document locking, to me it looks more like a case of abstraction only for abstraction's sake, and without any higher-order purpose or goal to doing it.

In the example I showed, the abstraction can be justified because it promotes and supports the separation of code based on whether it is dependent on the application layer, or only dependent on the database layer.

That's a good a example of where abstraction has a higher purpose, but I see no higher-order purpose or objective to your abstraction of document locking and transactions separately, and going back to the example in your first post, where you use all three of those abstractions, the only purpose it serves other than to avoid repetition of common error handling code, is to obfuscate the purpose and function of the application-level code.

Can you demonstrate a good reason for/purpose to abstracting transactions and document locking (separately, rather than all-inclusively as my example does) ?

In addition, abstracting try/catch (as opposed to just exposing a shared API that has common error handling code that can be called explicitly from a catch{} block), is a problem, because it does not allow the exception handling code in the catch block to access objects in the local scope of the called action.

I have plenty of code here that uses try/catch, where the code in the catch{} block must access local variables or member variables of the class containing the method that contains the try/catch block, and if I were to abstract out the try/catch as you do, that would not be possible.


TheMaster

  • Guest
Re: Wrapping command actions
« Reply #18 on: May 09, 2012, 04:03:45 PM »

I'm considering changing my theme over to the Dark Side (from default).   :mrgreen:

/offtopic

Cheers! :beer:

I just looked at that, and don't see how medium-dark blue text on a black background is any better than yellow or lime on a white background :laugh:.

My eyesight is far from perfect and for me, higher-contrasting colors is necessary (and is one reason why I think Autodesk's website with medium-gray text on black background is one of the most poorly-designed websites on the internet, especially for its blatant disregard for the visually-impaired).

I use a dark background in VS for the same reason I use a black background in AutoCAD - because it makes it easier to resolve different shades of the same color, and therefore allows me to use more colors.


Jeff H

  • Needs a day job
  • Posts: 6150
Re: Wrapping command actions
« Reply #19 on: May 09, 2012, 04:07:27 PM »
I like my colors to resemble my code so I set my VS background to doodoo brown.
 
 
 

BlackBox

  • King Gator
  • Posts: 3770
Re: Wrapping command actions
« Reply #20 on: May 09, 2012, 04:08:09 PM »

I'm considering changing my theme over to the Dark Side (from default).   :mrgreen:

/offtopic

Cheers! :beer:

I just looked at that, and don't see how medium-dark blue text on a black background is any better than yellow or lime on a white background :laugh:.

Laughably, I am very comfortable with the VLIDE colors, and this (the Dark Side theme) is the best that I've found thus far... Not that it's perfect, I'd still want to make some personalized changes. Just saying.

My eyesight is far from perfect and for me, higher-contrasting colors is necessary (and is one reason why I think Autodesk's website with medium-gray text on black background is one of the most poorly-designed websites on the internet, especially for its blatant disregard for the visually-impaired).

I use a dark background in VS for the same reason I use a black background in AutoCAD - because it makes it easier to resolve different shades of the same color, and therefore allows me to use more colors.

My third and final time posting this (today), but so applicable it cannot be avoided :lol:....


"How we think determines what we do, and what we do determines what we get."

Jeff H

  • Needs a day job
  • Posts: 6150
Re: Wrapping command actions
« Reply #21 on: May 09, 2012, 05:33:28 PM »
It maybe would have been nice if AutoDesk added property to CommandAttribute that told it to add and start a transaction before calling you method and commiting it when control was returned and maybe exposed CancelCommit or you would have access to it with TopTransaction property.
 
 
Maybe for 5.0 if possible with Roslyn project to call dispose on all newly created Dbobjects that are not added to the Database when they fall out of scope.

TheMaster

  • Guest
Re: Wrapping command actions
« Reply #22 on: May 10, 2012, 07:08:57 PM »
It maybe would have been nice if AutoDesk added property to CommandAttribute that told it to add and start a transaction before calling you method and commiting it when control was returned and maybe exposed CancelCommit or you would have access to it with TopTransaction property.


The problem there is that I have very few commands implemented in .NET
that unconditionally start and commit a transaction. Most of those that do,
only do after all user interaction is done, and the user hasn't decided to
bail out.  Ditto for document locking.

I think one would be hard-pressed to find very many cases where either or
both a transaction and a document lock wrap the entire implementation
of a .command method.

One problem with the OP's wrapper for try/catch is that it doesn't rethrow the
exception, which means that it could only be useful when it wraps the entire
command implementation. If it were used in any other context, and there was
more code following the call to execute the action, that code would run as if no
error had occurred.

The .NET  Application object has an event that fires when an exception is raised
and is not handled, which is how most do exception logging, but that event does
not get fired in AutoCAD and I think that's a notable deficiency, because if there
was an event that could be used to intercept unhandled exceptions, it would be
a far better way to solve the problem which the OP's wrapper tries to solve.

Quote
Maybe for 5.0 if possible with Roslyn project to call dispose on all newly created Dbobjects that are not added to the Database when they fall out of scope.

It's not when a DBObject wrapper goes out of scope, it's when it becomes
unreachable through code (and becomes eligible for garbage collection) that
would be the trigger.  If there's only one variable on the stack referencing a
DBObject, then it is simple, but the problem is there's no easy way to find out
when an object has become unreachable, and compiler extensions you speak
of (Rosyln) that allow you to hook into the compilation process won't really
help with that problem. The garbage collector itself doesn't even know when
an object becomes unreachable, it must scan the heap and stack each time it
does a sweep to find out.