Newbie Question Result

I posted a query earlier about retreivin the colour information from an
object, thankyou everybody for your help, and based on the first reply I
wrote the following which sets the current layer and colour to that of the
selected object. Tell me what you think, it may not be the most efficient
but it works, and I am just learning.

(defun c:setla()
(setq ss (entget (car (entsel "Please choose an Object:"))))
(setq
colr (cdr (assoc 62 ss))
lay (cdr (assoc 8 ss))
)
(setq a (null colr))
(setq b T)

(if (= a b)
(command "-color" "bylayer")
(command "-color" colr)
)
(command "-layer" "s" lay "")
)

 

Tell me what you think, it may not be the most efficient but it works, and
I am just learning.

Congratulations! A couple of suggestions:

Get in the habit of always declaring your variables local. If you don't,
eventually you'll create a nightmare.

Think about bulletproofing, even for your own use and definitely if you're
ever going to share a routine. Your lisp crashes if the user fails to select
an object.

Using the command function to do things has its pros and cons. It's "easy"
programmatically but has limitations. Usually it's better to avoid it if you
can. In this case, it takes two U's to undo your lisp, because you've run
two commands. They ought to be wrapped in an undo group so they can be
undone with a single U, as the user would expect. Since you're manipulating
variables, you can also do it by means of setvar instead of command. But
this also requires an undo group, because without using a command, an undo
mark isn't automatically set.

(defun c:setla (/ obj colr lay)
(command "undo" "begin")
(if (setq obj (entsel "\nPlease choose an Object:"))
(progn
(setq
obj (entget (car obj))
colr (cdr (assoc 62 obj))
lay (cdr (assoc 8 obj)))
(if (null colr)
(setq colr "256")
(setq colr (itoa colr)))
(setvar "cecolor" colr)
(setvar "clayer" lay)))
(command "undo" "end")
(princ))

 

Hi Tom,

Thanks for your help, I have sat here studying your code for the last 5min
and I think I can understand everything that you have done. The undo command
is very useful.

Craig

 

Thanks for your help, I have sat here studying your code for the last 5min

It can be very important. In another thread here, a person asks about a lisp
which runs dozens of command functions, and each one has to be undone
separately! Remember, the user will think of your routine as a custom
"command" and they'll expect it to undo and redo just like built-in
commands.

On the other hand, if you do things in "pure" lisp, without calling
commands, you don't get any automatic U marks. If you run functions like
this for a while, then do a U, Acad will undo all the way back to your last
built-in command! So either way it's best to always wrap your work in an
undo group.

 

I was thinking the same thing about bulletproofing. My solution is usually
along the lines of:

(defun C:SETLA ( / obj)
(setq obj (entsel "\nPlease choose an Object:"))
(while (= obj nil)
(princ "\n...\n...\n... You failed to select anything! Try Again.")
(setq obj (entsel "\nPlease choose an Object:"))
)
(setq obj (entget (car obj)))
)

The (while) keeps you picking until you finally select something or > if you
have to < hit ESC. Just another way to skin the cat.


chris