.NET

Reply
Distinguished Contributor
area51visitor
Posts: 119
Registered: ‎03-05-2011
Message 1 of 19 (434 Views)
Accepted Solution

Will the best code reviewer please stand up

434 Views, 18 Replies
03-05-2013 08:02 PM

Just hoping someone will review my code below and offer advice for improvements. Basically want to learn. Thanks!!!

 

// send wipeouts to back

namespace BCCWIPEBACK
{
    public class DraftingTools
    {

        [CommandMethod("BCC:WBACK")]

        public static void BCCWIPETOBACK()
        {
            Document doc = Application.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Transaction tr = db.TransactionManager.StartTransaction();

            using (tr)
            {
                BlockTable bt = tr.GetObject(db.BlockTableId, OpenMode.ForWrite) as BlockTable;
                foreach (ObjectId objId in bt)
                {
                    BlockTableRecord btr = objId.GetObject(OpenMode.ForWrite) as BlockTableRecord;

                    foreach (ObjectId btrObjId in btr)
                    {
                        Entity ent = btrObjId.GetObject(OpenMode.ForWrite) as Entity;
                        if (ent is Wipeout)
                        {
                            DrawOrderTable drawOrder = tr.GetObject(btr.DrawOrderTableId, OpenMode.ForWrite) as DrawOrderTable;
                            ObjectIdCollection ids = new ObjectIdCollection();
                            ids.Add(ent.ObjectId);
                            drawOrder.MoveToBottom(ids);  //move the selected entity so that entity is drawn in the beginning of the draw order.
                        }
                    }
                } tr.Commit();
            }
        }
    }
}

 

- Brian
"Very funny, Scotty. Now beam down my clothes."

Please compare:

namespace BCCWIPEBACK
{
public class DraftingTools
{
[CommandMethod("BCC:WBACK")]
public static void BCCWIPETOBACK()
{
Document doc = Application.DocumentManager.MdiActiveDocument;
Database db = doc.Database;
using (Transaction tr = db.TransactionManager.StartTransaction()) {
BlockTable bt = tr.GetObject(db.BlockTableId, OpenMode.ForRead) as BlockTable;
if (bt != null) {
foreach (ObjectId objId in bt) {
BlockTableRecord btr = objId.GetObject(OpenMode.ForRead) as BlockTableRecord;
if (btr != null) {
ObjectIdCollection ids = new ObjectIdCollection();
foreach (ObjectId btrObjId in btr) {
if (btrObjId.ObjectClass.IsDerivedFrom(RXClass.GetClass(typeof(Wipeout)))) {
ids.Add(btrObjId);
}
//////////////////////////////////////////////////////////////////////////////////
// Old versions of AutoCAD .NET API has not ObjectId.ObjectClass method. So:
//////////////////////////////////////////////////////////////////////////////////
// Entity ent = btrObjId.GetObject(OpenMode.ForRead) as Entity;
// if (ent != null && ent is Wipeout) {
// ids.Add(btrObjId);
// }
}
if (ids.Count > 0) {
DrawOrderTable drawOrder =
tr.GetObject(btr.DrawOrderTableId, OpenMode.ForWrite) as DrawOrderTable;
if (drawOrder != null) {
// Why MoveToBottom()? In this case wipeouts
// do not hide entities. Maybe use MoveToTop() instead?
// drawOrder.MoveToTop(ids);
drawOrder.MoveToBottom(ids);
}
}
}
}
}
tr.Commit();
}
doc.Editor.Regen();
}
}
}

 

Moderator
Alexander.Rivilis
Posts: 1,416
Registered: ‎04-09-2008
Message 2 of 19 (406 Views)

Re: Will the best code reviewer please stand up

03-06-2013 01:32 AM in reply to: area51visitor

Please compare:

namespace BCCWIPEBACK
{
  public class DraftingTools
  {
    [CommandMethod("BCC:WBACK")]
    public static void BCCWIPETOBACK()
    {
      Document doc = Application.DocumentManager.MdiActiveDocument;
      Database db = doc.Database;
      using (Transaction tr = db.TransactionManager.StartTransaction()) {
        BlockTable bt = tr.GetObject(db.BlockTableId, OpenMode.ForRead) as BlockTable;
        if (bt != null) {
          foreach (ObjectId objId in bt) {
            BlockTableRecord btr = objId.GetObject(OpenMode.ForRead) as BlockTableRecord;
            if (btr != null) {
              ObjectIdCollection ids = new ObjectIdCollection();
              foreach (ObjectId btrObjId in btr) {
                if (btrObjId.ObjectClass.IsDerivedFrom(RXClass.GetClass(typeof(Wipeout)))) {
                  ids.Add(btrObjId);
                }
                //////////////////////////////////////////////////////////////////////////////////
                //  Old versions of AutoCAD .NET API has not ObjectId.ObjectClass method. So:
                //////////////////////////////////////////////////////////////////////////////////
//                Entity ent = btrObjId.GetObject(OpenMode.ForRead) as Entity;
//                if (ent != null && ent is Wipeout) {
//                  ids.Add(btrObjId);
//                }
              }
              if (ids.Count > 0) {
                DrawOrderTable drawOrder = 
                   tr.GetObject(btr.DrawOrderTableId, OpenMode.ForWrite) as DrawOrderTable;
                if (drawOrder != null) {
                  // Why MoveToBottom()? In this case wipeouts
                  // do not hide entities. Maybe use MoveToTop() instead?
                  // drawOrder.MoveToTop(ids);  
                  drawOrder.MoveToBottom(ids);
                }
              }
            }
          } 
        }
        tr.Commit();
      }
      doc.Editor.Regen();
    }
  }
}

 


Пожалуйста не забывайте про Утвердить в качестве решения! Утвердить в качестве решения и Give Kudos!Баллы
Please remember to Accept Solution! Accept as Solution and Give Kudos!Kudos

Distinguished Contributor
area51visitor
Posts: 119
Registered: ‎03-05-2011
Message 3 of 19 (390 Views)

Re: Will the best code reviewer please stand up

03-06-2013 04:02 AM in reply to: area51visitor

Thanks Alexander (Alex ok?)!  You are awesome!

 

I really appreciate it... the "if" statements obviously stand out (on my phone that is lol), but will definately study it more the next few days.


Really appreciate your help.

- Brian
"Very funny, Scotty. Now beam down my clothes."
Moderator
Alexander.Rivilis
Posts: 1,416
Registered: ‎04-09-2008
Message 4 of 19 (389 Views)

Re: Will the best code reviewer please stand up

03-06-2013 04:07 AM in reply to: area51visitor

bcinnv wrote:

...Alex ok?...


OK!


Пожалуйста не забывайте про Утвердить в качестве решения! Утвердить в качестве решения и Give Kudos!Баллы
Please remember to Accept Solution! Accept as Solution and Give Kudos!Kudos

Distinguished Mentor
BlackBox_
Posts: 772
Registered: ‎02-25-2013
Message 5 of 19 (351 Views)

Re: Will the best code reviewer please stand up

03-06-2013 01:48 PM in reply to: Alexander.Rivilis

Hi Alexander,

 

Would it not be better to use StartOpenCloseTransaction() in lieu of StartTransaction()?

 

Also, since the Database is being modified, shouldn't a DocumentLock wrap the Transaction's using statement?



"Potential has a shelf life." - Margaret Atwood


Autodesk Exchange Apps ~ Autoloader ~ AutoCAD Security

Moderator
Alexander.Rivilis
Posts: 1,416
Registered: ‎04-09-2008
Message 6 of 19 (339 Views)

Re: Will the best code reviewer please stand up

03-06-2013 02:59 PM in reply to: BlackBox_

Would it not be better to use StartOpenCloseTransaction() in lieu of StartTransaction()?


This is a complex question. Possible and so and so. My preference - native ObjectARX and smart pointers. :smileyhappy:


Also, since the Database is being modified, shouldn't a DocumentLock wrap the Transaction's using statement?

No. As far as this function is the command handler, and this command working in document context (has not CommandFlag.Session flag) In this case, AutoCAD itself locks the document before command start and unlocks after its stop.


Пожалуйста не забывайте про Утвердить в качестве решения! Утвердить в качестве решения и Give Kudos!Баллы
Please remember to Accept Solution! Accept as Solution and Give Kudos!Kudos

Distinguished Mentor
BlackBox_
Posts: 772
Registered: ‎02-25-2013
Message 7 of 19 (332 Views)

Re: Will the best code reviewer please stand up

03-06-2013 03:18 PM in reply to: Alexander.Rivilis

Thanks for that clarification, Alexander... I did have a separate question, though, if I may?

 

Why iterate the entire BlockTable, when one could iterate a simple SelectionSet?

 

Pseudo-code (tested):

 

using Autodesk.AutoCAD.ApplicationServices;
using Autodesk.AutoCAD.DatabaseServices;
using Autodesk.AutoCAD.EditorInput;
using Autodesk.AutoCAD.Runtime;

using acApp = Autodesk.AutoCAD.ApplicationServices.Application;

namespace Autodesk.Forums.PseudoCode.DraftingTools
{
    public class Commands
    {
        [CommandMethod("WBACK")]
        public static void WipeOutToBack()
        {
            Document doc = acApp.DocumentManager.MdiActiveDocument;
            Database db = doc.Database;
            Editor ed = doc.Editor;

            TypedValue[] tvs = new TypedValue[]
            {
                new TypedValue((int)DxfCode.Start, "WIPEOUT")
            };

            SelectionFilter filter = new SelectionFilter(tvs);

            PromptSelectionResult psr = ed.SelectAll(filter);

            if (psr.Status == PromptStatus.OK)
            {
                using (Transaction trans =
                        db.TransactionManager.StartOpenCloseTransaction())
                {
                    SelectionSet ss = psr.Value;

                    ObjectId[] ids = ss.GetObjectIds();

                    foreach (ObjectId id in ids)
                    {
                        Entity ent =
                            trans.GetObject(id, OpenMode.ForRead) as Entity;

                        BlockTableRecord btr =
                            trans.GetObject(ent.BlockId, OpenMode.ForRead) as BlockTableRecord;

                        DrawOrderTable drawOrder =
                            trans.GetObject(btr.DrawOrderTableId, OpenMode.ForWrite) as DrawOrderTable;

                        ObjectIdCollection idToMove = new ObjectIdCollection();

                        idToMove.Add(id);

                        drawOrder.MoveToBottom(idToMove);
                    }

                    ed.WriteMessage("\n** Wipeout(s) sent to back ** ");

                    trans.Commit();
                }
            }

            else
                ed.WriteMessage("\n** No wipeout(s) found ** ");
        }
    }
}

 

 



"Potential has a shelf life." - Margaret Atwood


Autodesk Exchange Apps ~ Autoloader ~ AutoCAD Security

Valued Mentor
DiningPhilosopher
Posts: 370
Registered: ‎05-06-2012
Message 8 of 19 (316 Views)

Re: Will the best code reviewer please stand up

03-06-2013 06:19 PM in reply to: BlackBox_

BlackBoxCAD wrote:

Thanks for that clarification, Alexander... I did have a separate question, though, if I may?

 

Why iterate the entire BlockTable, when one could iterate a simple SelectionSet?

 

Pseudo-code (tested):......

 

<snip> 

 


 

You might want to review your code.

 

First, you open the same DrawOrderTable for every entity in the selection.

 

Second, you are calling MoveToBottom() once for every entity in the selection.

 

Third, you are not modifying the WIPEOUT objects in the selection, and don't need to open them in the first place, because they are all WIPEOUT objects.

 

So....

 

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

namespace Autodesk.AutoCAD.ApplicationServices.MyExtensions
{
   public static class WipeOutOrderExample
   {
      [CommandMethod( "WIPEOUTORDER" )]
      public static void WipeOutOrderCommand()
      {
         Document doc = Application.DocumentManager.MdiActiveDocument;
         Editor ed = doc.Editor;

         TypedValue[] array = new TypedValue[] {
            new TypedValue(0, "WIPEOUT")};

         PromptSelectionResult psr = ed.SelectAll( new SelectionFilter( array ) );
         if( psr.Status != PromptStatus.OK || psr.Value.Count == 0 )
         {
            ed.WriteMessage( "\nNo editable WIPEOUT objects found." );
            return;
         }

         ed.WriteMessage( "\n{0} WIPEOUTs found,", psr.Value.Count );
         PromptKeywordOptions pko = new PromptKeywordOptions( "\nMove to Top or Bottom?" );
         pko.Keywords.Add( "Top" );
         pko.Keywords.Add( "Bottom" );
         pko.Keywords.Default = "Bottom";
         PromptResult pr = ed.GetKeywords( pko );
         if( pr.Status != PromptStatus.OK )
            return;

         bool top = pr.StringResult == "Top";

         using( Transaction tr = doc.TransactionManager.StartTransaction() )
         {
            ObjectIdCollection ids = new ObjectIdCollection( psr.Value.GetObjectIds() );

            BlockTableRecord btr = (BlockTableRecord)
               tr.GetObject( doc.Database.CurrentSpaceId, OpenMode.ForRead );

            DrawOrderTable dot = (DrawOrderTable)
               tr.GetObject( btr.DrawOrderTableId, OpenMode.ForWrite );

            if( top )
               dot.MoveToTop( ids );
            else
               dot.MoveToBottom( ids );

            tr.Commit();
            doc.Editor.WriteMessage( "Modified draw order of {0} WIPEOUTs", ids.Count );
         }
      }
   }
}

 

Distinguished Contributor
area51visitor
Posts: 119
Registered: ‎03-05-2011
Message 9 of 19 (311 Views)

Re: Will the best code reviewer please stand up

03-06-2013 06:40 PM in reply to: area51visitor

Thank you! sorry... I should clarify the purpose of the code is to deal with a glitch I've had since I moved to windows 7 / 64bit system, and Civil 3d. Wipeouts randomly jump to the top of the objects. Such as textmasks. Instead of hiding what's under the text...the draw order randomly reverses, and hides everything. Often it's a problem due to locked layers, but that's not always the case.

 

When I manually set the draw order for the wipeouts by selecting them all, I no longer have that problem.  What I've been doing is sending the wipeouts to the back, then my xrefs. It's worked well so far. 

 

Sorry for the confusion.

- Brian
"Very funny, Scotty. Now beam down my clothes."
Distinguished Mentor
BlackBox_
Posts: 772
Registered: ‎02-25-2013
Message 10 of 19 (306 Views)

Re: Will the best code reviewer please stand up

03-06-2013 07:03 PM in reply to: DiningPhilosopher

DiningPhilosopher wrote:



 

You might want to review your code.

 

First, you open the same DrawOrderTable for every entity in the selection.

 

Second, you are calling MoveToBottom() once for every entity in the selection.

 

...

 

//<snip>

How very inefficient of me (oops); thanks for the correction....

 

That was a kludge thrown together for me to learn from (and that I have, thank you) right as I was leaving the office, as every single ADN DevBlog example only showed how to modify the draw order for single entity selection... Couldn't find one modifying SelectionSet.

 

Admittedly, I overlooked passing SelectionSet.GetObjectIds() to DrawOrderTable.MoveTo*(). Thanks for the clarification, and code as an example.

 

Now that that's out of the way, I am correct in that (logically) it is better to perform this task on a valid SelectionSet, rather than iterating the BlockTable, no?

 

Cheers!

 

Edit to add:

 

"Third, you are not modifying the WIPEOUT objects in the selection, and don't need to open them in the first place, because they are all WIPEOUT objects."

 

The code I posted (never mind the duplication of work) _did_ modify the Wipeout entities (not express tools text masks, WIpeouts)... My test was performed on a line drawn with a Wipeout drawn over the top such that part of the line was no longer visible. The code (albeit innefficiently) correctly moved the Wipeout(s) to the back so that the line was fully visible.

 

If I've misunderstood your point, please clarify.



"Potential has a shelf life." - Margaret Atwood


Autodesk Exchange Apps ~ Autoloader ~ AutoCAD Security

Announcements
Are you familiar with the Autodesk Expert Elites? The Expert Elite program is made up of customers that help other customers by sharing knowledge and exemplifying an engaging style of collaboration. To learn more, please visit our Expert Elite website.
Need installation help?

Start with some of our most frequented solutions or visit the Installation and Licensing Forum to get help installing your software.