Topic: Setting Layer Current

[Kerry]

Setting Layer Current
Glenn should recognise this

Any Comments ? ?

Code: [Select]

// Setting Layer Current
// AC2006 from MSVC#2005
// by kwb 2005Nov23

#region using declarations
using System;

////// Assembly acdbmgd .. ObjectDBX.NET Managed Wrapper
////// Assembly acmgd .. Autocad.NET Managed Wrapper

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

using pAcadApp = Autodesk.AutoCAD.ApplicationServices.Application;

#endregion

[assembly: CommandClass(typeof(KWBTesting.KDUBTestLayer))]
[assembly: ExtensionApplication(typeof(KWBTesting.KDUBTestLayer))]


namespace KWBTesting
{
    public class KDUBTestLayer : IExtensionApplication
    {
        [CommandMethod("LayerTest", CommandFlags.Modal)]
        public void test()
        {
            Document pDoc = pAcadApp.DocumentManager.MdiActiveDocument;
            Database pDb = pDoc.Database;
            Editor pEd = pDoc.Editor;

            string layerName = "Test001";

            ObjectId layerId = ObjectId.Null;

            using (Transaction pTr = pDb.TransactionManager.StartTransaction())
            {
                try
                {
                    LayerTable pLayerTable = (LayerTable)pTr.GetObject(pDb.LayerTableId, OpenMode.ForRead);

                    if (pLayerTable.Has(layerName))
                    {
                        layerId = pLayerTable[layerName];
                        if (layerId.IsErased)
                        {
                            LayerTableRecord pExLtr = (LayerTableRecord)pTr.GetObject(layerId, OpenMode.ForWrite, true);
                            pExLtr.Erase(false);
                        }

                    } else // create it...
                    {
                        LayerTableRecord pNewLTR = new LayerTableRecord();

                        pNewLTR.Name = layerName;
                        pLayerTable.UpgradeOpen();
                        layerId = pLayerTable.Add(pNewLTR);
                        pTr.AddNewlyCreatedDBObject(pNewLTR, true);
                    }

                    pDb.Clayer = layerId;
                    pTr.Commit();
                   
                } catch (Autodesk.AutoCAD.Runtime.Exception acEx)
                {
                    pEd.WriteMessage("\nError: " + acEx.Message);
                    return;
                }
             }
        }
        #region IExtensionApplication Members

        public void Terminate()
        {
            // TODO:  Add KDUBTestLayer.Terminate implementation
        }

        public void Initialize()
        {
            // TODO:  Add KDUBTestLayer.Initialize implementation
        }

        #endregion

    }
}


mods : {thanks Glenn}
Commit statement relocated :
finally block removed
return added to catch block.

[Kerry]

now, a question

Should/could these be reversed ?

Code: [Select]

                    pTr.Commit();
                    pDb.Clayer = layerId;

[Glenn R]

Hehe...

1. YES - They most DEFINATELY should be reversed. Generally, keep everything inside the transaction boundary.

2. The 'finally' clause is redundant. If an exception is thrown, it's gets caught in your 'catch' block - SKIPPING the call to 'Commit()'.
Your code then carries on and in this case there is nothing else. The important point here is that you then pass out of the scope of your 'using'
statement WITHOUT having Commit called on the transaction. This is when Dispose is called on your transaction object which is actually a call to abort,
seeing as you skipped Commit. I prefer to have an explicit 'return' statement in the catch block here, but that's personal preference.

3. The default indexer on symbol tables ,at least, is returning ERASED records I found. I will discuss more after you try this Kerry:

Run your code on a drawing that doesn't have your layer in it.
Then purge that layer.
Then run your code again.

Repeat several times with different combos - I think you will find it fails...not certain though.

Post your results.

Cheers,
Glenn.

PS - Excellent style btw 

[Kerry]

Hi Glenn

I changed the commit location in the posted code.  I thought that was the case .. Thanks.
Changed the others as suggested also ..

Tried your test .. no problems.
.. BUT I do know that there was a problem at one stage, thats why I used the .isErased conditional. I tried Deleting the Layer from the Layer Manager, as well as purging. I'll try destruction testing later

Thanks,
Kerry

[Glenn R]

Kerry, thanks for the kind words.

Riddle me this: 

Let's say you're iterating the BLOCK table, come across an entry that's ERASED and then un-erase it.......think about all the dependencies on a block table record - extension dictionaries etc.........see my point? (Blatant un-erasing is not a good thing.

I'll try to dig up some code i was using tat found this nasty little error......I did post on the adesk .net newsgroup about this and Tony T confirmed my suspicions...see if you can find  the post (there aren't a lot by me there, so the search should be small).

Interesting discussion - good topic.

BTW, the 'return' statement thing is just a matter of style - I prefer to bail out as soon as possible. Hence, you'll never see me having big long 'if' statments like
If (this)
{
// Do a LOT of mojo here...
}

It will be:
If (!this)
 return;


Cheers,
Glenn.

[Draftek]

Thanks Guys,

Too busy to really reply, but following.

Hope to contribute soon.

[Kerry]

>>> .........see my point? (Blatant un-erasing is not a good thing.

it was a selective un-erase, but Yep, I sure do see the point.

So perhaps the correct algorithm would be :

if the layer record exists and iserased
then make a new layer with a unique prefix to the required layer name and return the id .. and advise the user accordingly.

else if just exists return id

else if not existing, make the named layer and return the id