Jump to content

please critique this


AAONCAD

Recommended Posts

I have accesed the original code for a series commands that perform simple operations; they operate fine in Acad 14 but, will not execute in 2010. any recognizable problems? i.e the one listed "MC27" is supposed to change text to read MC27 but it doesn't.

(defun c:27 ()                                                     ;ECN MC27 REV BENDS ADD DECIMALS (6/9/08)
 (setvar "cmdecho" 0)
 (setq obj (entsel "\nPick Location :"))
 (setq a "MC27" )
 (setq j (strcat a))
 (command "change" obj "" "" "" "" "" "" j))

 

also I have a PDF attachment showing an option needed through DCL

it's 263 kb and I can't attach it through Cad tutor. Will anyone interested in taking a look please email me at mcclain@aaon.com

thank you for any reply's

Link to comment
Share on other sites

  • Replies 21
  • Created
  • Last Reply

Top Posters In This Topic

  • Lee Mac

    9

  • flowerrobot

    5

  • AAONCAD

    5

  • CADMASTER1128

    3

Top Posters In This Topic

A few things,

  • There is no need for:

(setq j (strcat a))

You are only dealing with one string, hence you don't need to concatenate it.

 

  • You have not localised your variables, this is not good programming practice, the funciton definition should look something like:

(defun c:27 (/ obj a)

 

  • I would recommend that you use more descriptive variable names, "obj" is OK, but refrain from using single letters. The code will then be easier to read.

  • You will need to use:

(car (entsel "\nPick Text: "))

 

As
entsel
returns a list with two elements, the first being the entity name and the second being the pick point.

 

  • I would refrain from using the change command to change your text, instead, look into entmod.

  • IMPORTANT: you have not reset the system variable CMDECHO. Whilst this may not be noticeable too much with CMDECHO, it is very bad practice and can lead to undesired results, especially when using other sys vars.

  • I would use a conditional statement, like the IF function, to allow for a missed pick, or no user input.

  • Use (princ) at the end of the program to exit the program cleanly, i.e. suppress the last function return.

Hope this helps,

 

Lee

Link to comment
Share on other sites

thank you Lee for your tips. If you would take this code load it and type a string of text(anything) and input 27 into command line and see why it will not change the text to MC27.

 

(defun c:27 (/ obj a)                               ;ECN MC27 REV BENDS ADD DECIMALS (6/9/08)
 (setvar "if" 0)
 (car (entsel "\nPick Location: "))
 (setq a ("MC27")
 (setq j (strcat a)
 (command "entmod" obj "" "" "" "" "" "" j))
 (princ)

Link to comment
Share on other sites

hello, thanks for your interest.

 

My goal is to take any random text string and enter 27 for then command; it the prompts user to pick a point, and then changes text to read MC27

Link to comment
Share on other sites

Mate nice effort, but i belive lee's ment some thing more like this

 

 

(defun c:27 (/ ss)
(setvar "cmdecho" 0) 
(While (not ss) ;Entsel lets you only have one shot at selecting and item, So while lets you have as many attemps
(setq ss (entget (car (entsel "\nSelect text :")))))
(if (= (cdr (assoc 8 ss)) "text") ;Useing a If statement to make sure the item is text.
(entmod ;Entmod is a way of altering entites,
(subst (cons 1 "MC27") (assoc 1 ss) ss)) ;I used Subst, to substatue the text already there,
(Princ "You did not select text")
)
(setvar "cmdecho" 1) ; Turn cmdecho back on.
(princ)
)

Link to comment
Share on other sites

Rather, I was thinking something like this:

 

(defun c:27 (/ ent)
 (if (and (setq ent (car (entsel "\nPick Text: ")))
          (wcmatch (cdr (assoc 0 (entget ent))) "*TEXT"))
   (entmod
     (subst
       (cons 1 "MC27") (assoc 1 (entget ent))
         (entget ent)))
   (princ "\n** Object is not Text **"))
 (princ))

Link to comment
Share on other sites

Flower,
  1. Entget will return an error if supplied with nil, so your while would not work.
  2. "0" is the DXF code for the entity type, not 8

:)

 

Ahh i didnt kno that, I normally would of done while looking for text but didnt cause im wack job

 

if you look at text & mtext they both refer to dxf 8 as text, and so i used that instead of a wcmatch

Link to comment
Share on other sites

Wow, I really am that dumb.

 

I forgot i have reators running to keep all text on text layer.

 

i really cannot belive that nothing fired that 8 is layer, And not some wounderouse dxf group that sum's what it type of item it is.

 

 

Wow

Link to comment
Share on other sites

thanks for that. Its not that i didnt know 8 is layer, its that well i dont know, I thought it was some randomly new dxf that would fix all my drama's

 

lol

Link to comment
Share on other sites

This may increase it further :P

 

(defun c:27 (/ ss)
 (vl-load-com)
 (if (setq ss (ssget '((0 . "*TEXT"))))
   (mapcar
     (function
       (lambda (Obj)
         (vla-put-TextString Obj "MC27")))
     (mapcar 'vlax-ename->vla-object
       (vl-remove-if 'listp
         (mapcar 'cadr (ssnamex ss))))))
 (princ))

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.


×
×
  • Create New...