Seeking Advice to Better Routine

[brianchapmandesign]

If you're willing, please offer advice on how to make this routine better. Thanks!

//get position of two blocks

namespace BlockPosition
{
    public class DraftingTools
    {
        [CommandMethod("BP")]
        public void BPosition()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;


            try
            {
                while (true)
                {
                    PromptEntityOptions peo = new PromptEntityOptions("\nSelect first block:");
                    peo.SetRejectMessage("\nMust be a block.");
                    peo.AddAllowedClass(typeof(BlockReference), false);
                    PromptEntityResult psr = ed.GetEntity(peo);
                    if (psr.Status != PromptStatus.OK)
                        return;

                    using (Transaction tr = db.TransactionManager.StartTransaction())
                    {
                        ObjectId brId = psr.ObjectId;

                        // Open the block reference
                        BlockReference br = (BlockReference)tr.GetObject(brId, OpenMode.ForRead);


                        var att1 = br.Position;
                        ed.WriteMessage("\n{0}, Position: {1}", br.Name, att1);

                    }

                    PromptEntityOptions peo2 = new PromptEntityOptions("\nSelect second block:");
                    peo2.SetRejectMessage("\nMust be a block.");
                    peo2.AddAllowedClass(typeof(BlockReference), false);
                    PromptEntityResult psr2 = ed.GetEntity(peo2);
                    if (psr2.Status != PromptStatus.OK)
                        return;

                    using (Transaction tr = db.TransactionManager.StartTransaction())
                    {
                        ObjectId brId2 = psr2.ObjectId;

                        // Open the block reference
                        BlockReference br2 = (BlockReference)tr.GetObject(brId2, OpenMode.ForRead);

                        var att2 = br2.Position;
                        ed.WriteMessage("\n{0}, Position: {1}", br2.Name, att2);
                    }
                }
            }
            catch (System.Exception e)
            { Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message)); } 
        }

    }
}

[_gile]

Hi,

While you're doing the same thing for both blocks (excepted the prompt message) you can factorize these statements in a private method.

To avoid starting a transaction at each selection, you'd embed the while loop within the transaction.

        [CommandMethod("BP")]
        public void BPosition()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;

            try
            {
                using (Transaction tr = db.TransactionManager.StartTransaction())
                {
                    while (true)
                    {
                        if (!GetBlockInfos(ed, "\nSelect first block: "))
                            break;
                        if (!GetBlockInfos(ed, "\nSelect second block: "))
                            break;
                    }
                    tr.Commit();
                }
            }
            catch (System.Exception e)
            {
                Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message));
            }
        }

        private bool GetBlockInfos(Editor ed, String message)
        {
            PromptEntityOptions peo = new PromptEntityOptions(message);
            peo.SetRejectMessage("\nMust be a block.");
            peo.AddAllowedClass(typeof(BlockReference), false);
            PromptEntityResult per = ed.GetEntity(peo);
            if (per.Status != PromptStatus.OK)
                return false;
            BlockReference br = (BlockReference)per.ObjectId.GetObject(OpenMode.ForRead);
            ed.WriteMessage("\n{0}, Position: {1}", br.Name, br.Position);
            return true;
        }

You can also avoid starting a transaction (there's nothing to undo) using the ObjectId.Open() method which have to be considered as "for advanced use only." rather than "obsolete".

        [CommandMethod("BP")]
        public void BPosition()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;

            try
            {
                while (true)
                {
                    if (!GetBlockInfos(ed, "\nSelect first block: "))
                        break;
                    if (!GetBlockInfos(ed, "\nSelect second block: "))
                        break;
                }
            }
            catch (System.Exception e)
            {
                Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message));
            }
        }

        private bool GetBlockInfos(Editor ed, String message)
        {
            PromptEntityOptions peo = new PromptEntityOptions(message);
            peo.SetRejectMessage("\nMust be a block.");
            peo.AddAllowedClass(typeof(BlockReference), false);
            PromptEntityResult per = ed.GetEntity(peo);
            if (per.Status != PromptStatus.OK)
                return false;
            using (BlockReference br = (BlockReference)per.ObjectId.Open(OpenMode.ForRead))
            {
                ed.WriteMessage("\n{0}, Position: {1}", br.Name, br.Position);
            }
            return true;
        }

[brianchapmandesign]

Merci Gilles, thorough, helpful. I'll use it to continue learning.

[brianchapmandesign]

I modified the code to do what it was actually intended to do. I still have to look at the better approaches you gave me though, I'm not sure they are possible in my specific scenerio; course I am a noob, and just learning C# now.

namespace BCCGRSCOMMAND
{
    public class DraftingTools
    {
        [CommandMethod("BCC:GRS")]
        public void BCCGRS()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;

            try
            {
                while (true)
                {
                    PromptEntityOptions peo = new PromptEntityOptions("\nSelect first block:");
                    peo.SetRejectMessage("\nMust be a block.");
                    peo.AddAllowedClass(typeof(BlockReference), false);
                    PromptEntityResult psr = ed.GetEntity(peo);
                    if (psr.Status != PromptStatus.OK)
                        return;

                    PromptEntityOptions peo2 = new PromptEntityOptions("\nSelect second block:");
                    peo2.SetRejectMessage("\nMust be a block.");
                    peo2.AddAllowedClass(typeof(BlockReference), false);
                    PromptEntityResult psr2 = ed.GetEntity(peo2);
                    if (psr2.Status != PromptStatus.OK)
                        return;


                    using (Transaction tr = db.TransactionManager.StartTransaction())
                    {
                        ObjectId brId = psr.ObjectId;

                        // Open the block reference
                        BlockReference br = (BlockReference)tr.GetObject(brId, OpenMode.ForRead);

                        int att1 = 0;
                        int att2 = 0;

                        //iterate through attributes but break after first
                        foreach (ObjectId attId in br.AttributeCollection)
                        {
                            // Open the attribute reference
                            AttributeReference att1obj = (AttributeReference)tr.GetObject(attId, OpenMode.ForRead);
                            att1 = Convert.ToInt32(att1obj.TextString);

                            break; //break the process at the first attrib
                        }

                        ObjectId brId2 = psr2.ObjectId;

                        // Open the block reference
                        BlockReference br2 = (BlockReference)tr.GetObject(brId2, OpenMode.ForRead);

                        //iterate through attributes but break after first
                        foreach (ObjectId attId2 in br2.AttributeCollection)
                        {
                            // Open the attribute reference
                            AttributeReference att2obj = (AttributeReference)tr.GetObject(attId2, OpenMode.ForRead);
                            att2 = Convert.ToInt32(att2obj.TextString);
                            break; //break the process at the first attrib
                        }

                        //listing the final results

                        var position1 = br.Position;                     
                        var position2 = br2.Position;                                 
                        var distcheck = position1.DistanceTo(position2);                      
                        int diff = Math.Abs(att1 - att2);                        
                        var slopecheck = (diff / distcheck);

                        ed.WriteMessage("\n{0}, Position: {1}", br.Name, position1);
                        ed.WriteMessage("\n{0}, Position: {1}", br2.Name, position2);
                        ed.WriteMessage("\nDistance: {0}", distcheck);
                        ed.WriteMessage("\nDifference: {0}", diff);
                        ed.WriteMessage("\nSlope: {0}", slopecheck);
                    }
                }
            }
            catch (System.Exception e)
            { Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message)); }
        }

    }
}

[DiningPhilosopher]

Contrary to the misformation being spread by one Autodesk employee, you should never Abort transactions that are used for purely read-only operations. 

Aborting a transaction has massive overhead relative to committing, and since nothing has been modified, there's no undo information to record..

Also, if by chance you were to acquire input while a transaction was active,and the user panned or zoomed the view, that's a change to the drawing, and aborting the transaction will cause it to be undone, which isn't very nice.

To demonstrate just how much more overhead aborting a transaction has,

you can try loading and running this:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Diagnostics;
using Autodesk.AutoCAD.Runtime;
using Autodesk.AutoCAD.DatabaseServices;
using Autodesk.AutoCAD.EditorInput;
using Autodesk.AutoCAD.ApplicationServices;

namespace Duh
{
   public static class AbortThisCommands
   {
      static int iterations = 1000;

      [CommandMethod( "ABORT_THIS" )]
      public static void AbortThisCommand()
      {
         Document doc = Application.DocumentManager.MdiActiveDocument;

         PromptIntegerOptions peo = new PromptIntegerOptions( "\nIterations: " );
         peo.AllowZero = false;
         peo.AllowNegative = false;
         peo.DefaultValue = iterations;
         peo.UseDefaultValue = true;
         PromptIntegerResult pir = doc.Editor.GetInteger( peo );
         if( pir.Status != PromptStatus.OK )
            return;
         iterations = pir.Value;

         Stopwatch st = Stopwatch.StartNew();
         for( int i = 0; i < iterations; i++ )
         {
            using( Transaction tr = doc.TransactionManager.StartTransaction() )
            {
               BlockTable bt = (BlockTable) tr.GetObject(
                  doc.Database.BlockTableId,
                  OpenMode.ForRead, false, false );
               tr.Abort();
            }
         }
         st.Stop();

         doc.Editor.WriteMessage( "\nTime to abort {0} transactions: {1} ms",
            iterations, st.ElapsedMilliseconds );
         
         st = Stopwatch.StartNew();
         for( int i = 0; i < iterations; i++ )
         {
            using( Transaction tr = doc.TransactionManager.StartTransaction() )
            {
               BlockTable bt = (BlockTable) tr.GetObject(
                  doc.Database.BlockTableId, 
                  OpenMode.ForRead, false, false );
               tr.Commit();
            }
         }
         st.Stop();

         doc.Editor.WriteMessage( "\nTime to commit {0} transactions: {1} ms",
            iterations, st.ElapsedMilliseconds );
      }
   }
}

[_gile]

Hi,

To add to what DinnerPhilosopher said, rather than starting a transaction at each loop, you'd start a single transaction and make your while loop within it.

As you're doing the same thing with both block selections, you can extract a single method to prompt the user for selecting a block, get the BlockReference object from the selection, check if the selected block has attributes and if the first one can be converted into an integer.

The method would return true if succeeded or false otherwise and set the values of a BlockReference and an integer out parameters.

        [CommandMethod("BP")]
        public void BPosition()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;

            try
            {
                using (Transaction tr = db.TransactionManager.StartTransaction())
                {
                    while (true)
                    {
                        BlockReference br1,  br2;
                        int att1, att2;

                        if (!TryGetBlockInfo(ed, "\nSelect first block: ", out br1, out att1))
                            break;

                        if (!TryGetBlockInfo(ed, "\nSelect second block: ", out br2, out att2))
                            break;

                        double distcheck = br1.Position.DistanceTo(br2.Position);
                        int diff = Math.Abs(att1 - att2);

                        ed.WriteMessage(
                            "\n{0}, Position: {1}\n{2}, Position: {3}\nDistance: {4}\nDifference{5}\nSlope: {6}",
                            br1.Name,
                            br1.Position,
                            br2.Name,
                            br2.Position,
                            distcheck,
                            diff,
                            diff / distcheck);
                    }
                    tr.Commit();
                }
            }
            catch (System.Exception e)
            {
                Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message));
            }
        }

        /// <summary>
        /// Prompt the user to select a block.
        /// </summary>
        /// <param name="ed">The current Editor.</param>
        /// <param name="message">The message to display.</param>
        /// <param name="br">The selected block reference.</param>
        /// <param name="value">The block first attribute value.</param>
        /// <returns>True if the user select a block reference wich first attribute value is an integer, false otherwise.</returns>
        private bool TryGetBlockInfo(Editor ed, string message, out BlockReference br, out int value)
        {
            br = null;
            value = -1;

            PromptEntityOptions peo = new PromptEntityOptions(message);
            peo.SetRejectMessage("\nMust be a block.");
            peo.AddAllowedClass(typeof(BlockReference), false);
            PromptEntityResult per = ed.GetEntity(peo);

            if (per.Status == PromptStatus.OK)
                return false;

            br = (BlockReference)per.ObjectId.GetObject(OpenMode.ForRead);
            if (br.AttributeCollection.Count == 0)
                return false;

            AttributeReference attRef =
                (AttributeReference)br.AttributeCollection[0].GetObject(OpenMode.ForRead);
            if (int.TryParse(attRef.TextString, out value))
                return true;

            return false;
        }

[brianchapmandesign]

That's awesome Gilles. Very helpful. Thank you!

[_gile]

Thanks.

There's a typo in the upper code:

repace:

if (per.Status == PromptStatus.OK)

with:

if (per.Status != promptStatus.OK)

Another way would be creating a little class (called BlockInfo here) to get the required infos. In this example, I used the ObjectId.Open() method rather than using a Transaction to get the block reference and its first attribute. In case of incorrect inputs (none attribute or not integer value) an exception is thrown.

        [CommandMethod("BI")]
        public void GetBlockInfos()
        {
            try
            {
                Editor ed = AcAp.DocumentManager.MdiActiveDocument.Editor;
                while (true)
                {
                    BlockInfo bi1 = BlockInfo.Select(ed, "\nSelect first block: ");
                    if (bi1 == null)
                        break;
                    BlockInfo bi2 = BlockInfo.Select(ed, "\nSelect second block: ");
                    if (bi2 == null)
                        break;
                    ed.WriteMessage(
                        "\n{0}, Position: {1}\n{2}, Position: {3}\nDistance: {4}\nDifference: {5}\nSlope: {6}",
                        bi1.Name,
                        bi1.Position,
                        bi2.Name,
                        bi2.Position,
                        bi1.DistanceTo(bi2),
                        bi1.DifferenceTo(bi2),
                        bi1.SlopeTo(bi2));
                }
            }
            catch(System.Exception e)
            {
                Application.ShowAlertDialog(String.Format("You Failed Horribly :P\n{0}", e.Message));
            }
        }

        class BlockInfo
        {
            // private field
            private int value;

            // constructor
            public BlockInfo(ObjectId id)
            {
                using (BlockReference br = (BlockReference)id.Open(OpenMode.ForRead))
                {
                    if (br.AttributeCollection.Count == 0)
                        throw new ArgumentException("None attribute");
                    using (AttributeReference attRef =
                        (AttributeReference)br.AttributeCollection[0].Open(OpenMode.ForRead))
                    {
                        if (!int.TryParse(attRef.TextString, out value))
                            throw new ArgumentException("Not an integer");
                    }
                    Name = br.Name;
                    Position = br.Position;
                }
            }

            // properties
            public Point3d Position { get; private set; }

            public string Name { get; private set; }

            public int AttValue { get { return value; } }

            // public instance methods
            public double DistanceTo(BlockInfo other)
            {
                return this.Position.DistanceTo(other.Position);
            }

            public int DifferenceTo(BlockInfo other)
            {
                return Math.Abs(value - other.AttValue);
            }

            public double SlopeTo(BlockInfo other)
            {
                return this.DifferenceTo(other) / this.DistanceTo(other);
            }

            // public static method
            public static BlockInfo Select(Editor ed, string message)
            {
                PromptEntityOptions peo = new PromptEntityOptions(message);
                peo.SetRejectMessage("\nMust be a block.");
                peo.AddAllowedClass(typeof(BlockReference), false);
                PromptEntityResult per = ed.GetEntity(peo);
                if (per.Status != PromptStatus.OK)
                    return null;
                return new BlockInfo(per.ObjectId);
            }
        }

[DiningPhilosopher]

---

@_gile wrote:

Hi,

To add to what DinnerPhilosopher said, rather than starting a transaction at each loop, you'd start a single transaction and make your while loop within it.

---

Hi Gile. 

The test code was designed to simulate how transactions might be used in other methods that may have to be called many times in a loop. In some cases, they may be reusable methods that may not assume a transaction is already running.  The code wasn't an example of how to use transactions, as you've noted, it would serve no purpose to start and end one on each loop iteration.

[_gile · Accepted solution]

Yes, I understand, Tony, I wanted to purpose the OP to use a single transaction AND commit it.

[brianchapmandesign]

Thank you gentleman!  You've been very helpful