Help with this simple code..

Hello everyone, I'd like some help finding the error in this code. I was given this routine from a colleague. Apparently it works fine. I've tried it and found no problems other then when exiting the routine. We always get the same error

error: bad argument type: lentityp nil

Can someone take a quick look and help me find what I am obviously overlooking. Thanks for your help.

(DEFUN C:CTM ()
(SETQ SOURCE_TEXT (ENTGET (CAR (ENTSEL "\NSELECT SOURCE TEXT: "))))
(WHILE
(SETQ DEST_TEXT (ENTGET (CAR (ENTSEL "\NSELECT DESTINATION TEXT: "))))
(SETQ DEST_TEXT (SUBST (ASSOC 1 SOURCE_TEXT) (ASSOC 1 DEST_TEXT) DEST_TEXT))
(ENTMOD DEST_TEXT)
(ENTUPD (CDR (ASSOC -1 DEST_TEXT)))
)
(PRINC)
)

Kind Regards,
Matthew Corbin

 

I don't get any errors, however, you should change the uppercase "N" to lowercase "n" as shown below (then it too won't show on the screen):


(DEFUN C:CTM ()
(SETQ SOURCE_TEXT (ENTGET (CAR (ENTSEL "\nSELECT SOURCE TEXT: "))))
(WHILE
(SETQ DEST_TEXT (ENTGET (CAR (ENTSEL "\nSELECT DESTINATION TEXT: "))))
(SETQ DEST_TEXT (SUBST (ASSOC 1 SOURCE_TEXT) (ASSOC 1 DEST_TEXT) DEST_TEXT))
(ENTMOD DEST_TEXT)
(ENTUPD (CDR (ASSOC -1 DEST_TEXT)))
)
(PRINC)
)


As a matter of fact, I would change ALL the text to lowercase with the possible exception of what you want shown on the command line.

 

In addition to Walt's suggestions, you shouldn't ever place anything
with (entsel), such as (car), since if the user doesn't select anything
(entsel) returns nil.

Ask for the selection, then check to see if the variable is not nil,
then do the (entget(car......

The (while) statement needs to be similarly worded:
(while (setq dest_text (entsel "\nBlah"))
(if dest_text
(progn
(do you changes.....

HTH,
Jeff

to lowercase "n" as shown below (then it too won't show on the screen):

possible exception of what you want shown on the command line.

 

In addition to Walt's suggestions, you shouldn't ever place anything

Following up on Jeff's comments:

(defun c:ctm (/ source_text dest_text)
(if (setq source_text (entsel "\nSelect source text: "))
(progn
(command "undo" "begin")
(setq source_text (entget (car source_text)))
(while (setq dest_text (entsel "\nSelect destination text: "))
(setq dest_text (entget (car dest_text)))
(entmod (subst (assoc 1 source_text) (assoc 1 dest_text) dest_text)))
(command "undo" "begin")))
(princ))

Note that you ought to set an undo group, and you should always declare
variables local to the function. You don't need entupd when it's not a
complex entity.

 

you need to check to see if something was selected, before trying to
manipulate it
hth
Mark

(defun c:ctm ()
(setq source_text (car (entsel "\nSelect source text: ")))
(if source_text
(progn
(setq source_text (entget source_text))
(while
(setq dest_text(car (entsel "\nSelect destination text: ")))
(if dest_text
(progn
(setq dest_text (entget dest_text))
(setq dest_text (subst (assoc 1 source_text) (assoc 1 dest_text)
dest_text))
(entmod dest_text)
(entupd (cdr (assoc -1 dest_text)))
)
)
);while
);progn
);if
(princ)
)

given this routine from a colleague. Apparently it works fine. I've tried it
and found no problems other then when exiting the routine. We always get the
same error

overlooking.

 

Mark, see my post from an hour earlier -- great minds think alike Except
you dodn't declare his variables local, and I'm pretty sure entupd isn't
necessary except for complex entities.

 

How about a little more error checking in case it is not a text object.

Code:

(defun c:ctm (/ source_text dest_text)
(if (setq source_text (entsel "\nSelect source text: "))
(progn
(setq source_text (entget (car source_text)))
(if (member (cdr (assoc 0 source_text)) '("TEXT" "MTEXT"))
(progn
(command "undo" "begin")
(while
(setq dest_text (entsel "\nSelect destination text: "))
(setq dest_text (entget (car dest_text)))
(if (member (cdr (assoc 0 dest_text)) '("TEXT" "MTEXT"))
(entmod
(subst (assoc 1 source_text)
(assoc 1 dest_text)
dest_text
)
)
(prompt "\nNot a Text object, Try Again.")
)
)
(command "undo" "end")
)
(alert "\nNot a Text object.")
)
)
)
(princ)
)

 

Here is one that does more error checking and matched the layer and has
an option to enter the text string.

Code:

;; Change the text by selecting the source or entering the new test
;;  sMadson 02/2004  http://www.smadsen.com/
(defun C:QC (/ ething 1thing entl nlay ntxt)
(initget "Type")
(cond
((setq ething (entsel "\nSelect text object to match or [Type]: "))
(cond
((= ething "Type") (setq ntxt (getstring T "\nNew text: ")))
((and (vl-consp ething)
(member (cdr (assoc 0 (setq entl (entget (car ething)))))
'("TEXT" "MTEXT")))
(setq ntxt (cdr (assoc 1 entl))
nlay (cdr (assoc 8 entl)))))))
(and ntxt (> (strlen ntxt) 0)
(while (setq 1thing (entsel "\nSelect text to change: "))
(setq entl (entget (car 1thing)))
(cond ((assoc 3 entl)
(princ "\nThis version only supports max. 250 character MTEXTs"))
((setq entl (subst (cons 1 ntxt) (assoc 1 entl) entl))
(if nlay
(setq entl (subst (cons 8 nlay) (assoc 8 entl) entl)))
(entmod entl)))))
(princ)
)

 

Good points. Without the validity check, it would crash if you picked
anything that didn't have a 1 group. I left that out for simplicity.

 

90% of coding is testing and validating data, I call it the 'knucklehead
factor'
BTW, the original code snippet did not localize variables. I've seen
so-called
coporate AutoCAD Support Admins writing code without localizing variables
and nested function names, another large potential for creating errors and
conflicts, which can take alot of time to track down. Sorry just a real
peeve
of mine.

 

right, there was something wrong with our server - didn't see yours or
wouldn't have bothered, didn't look at it nearly as close as you did!