my first lisp program (Plz chk)

Dear Sir,

Please chk. my program its ok or not

Areamm.lsp

Looks good. There's a lot of cleanup to be done, but you'll learn more about that as you advance.

The one thing I have to say is a problem I ran into when I was working at your level. You changed the osmode and then prompted to select a point. I did this a lot, and at the time, it made a great deal of sense. What I came to realize is that this is incredibly limiting, and actually, quite dangerous. Limiting because the point you want might not be the point you're allowed to snap to, and dangerous because if your program errors at the getpoint, it won't progress through the rest of the program... in short, it won't reset the osmode. That was the biggest issue I had. Every so often, I'd notice that my osmode was not what it was supposed to be.. because my functions had failed, and didn't reset. I have since completely avoided changing the osmode in my functions.

Good start otherwise. ^.^

Take note of my comments Sachindkini,

[b][color=RED]([/color][/b][b][color=BLUE]defun[/color][/b] c:CA2  [b][color=RED]([/color][/b][b][color=BLUE]/[/color][/b] *error* p1 p5 p6 a b c d e fn[b][color=RED])[/color][/b]

 [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] oldCM [b][color=RED]([/color][/b][b][color=BLUE]getvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b][b][color=RED])[/color][/b]
       oldos [b][color=RED]([/color][/b][b][color=BLUE]getvar[/color][/b] [b][color=#ff00ff]"OSMODE"[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
 
 [i][color=#990099];; Store the original values of the[/color][/i]
 [i][color=#990099];; system variables before you change them,[/color][/i]
 [i][color=#990099];; then you can reset them at the end.[/color][/i]
 [i][color=#990099];; I see you did this for the OSMODE, but it[/color][/i]
 [i][color=#990099];; is good practice to do it will all sys vars.[/color][/i]

 [b][color=RED]([/color][/b][b][color=BLUE]defun[/color][/b] *error* [b][color=RED]([/color][/b]msg[b][color=RED])[/color][/b]
   [b][color=RED]([/color][/b][b][color=BLUE]if[/color][/b] oldCM [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b] oldCM[b][color=RED])[/color][/b][b][color=RED])[/color][/b]
   [b][color=RED]([/color][/b][b][color=BLUE]if[/color][/b] oldos [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"OSMODE"[/color][/b] oldos[b][color=RED])[/color][/b][b][color=RED])[/color][/b]
   [b][color=RED]([/color][/b][b][color=BLUE]princ[/color][/b] msg[b][color=RED])[/color][/b]
   [b][color=RED]([/color][/b][b][color=BLUE]princ[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]

 [i][color=#990099];; Include a short error handler to reset the[/color][/i]
 [i][color=#990099];; System Variables, should the user hit Esc[/color][/i]
 [i][color=#990099];; during the program. Remember to localise this[/color][/i]
 [i][color=#990099];; function, as it is defined within the main function.[/color][/i]
 
 [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b] [b][color=#009900]0[/color][/b][b][color=RED])[/color][/b]
 [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"osmode"[/color][/b] [b][color=#009900]524[/color][/b][b][color=RED])[/color][/b]
 
 [b][color=RED]([/color][/b][b][color=BLUE]if[/color][/b] [b][color=RED]([/color][/b][b][color=BLUE]and[/color][/b] [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] p1 [b][color=RED]([/color][/b][b][color=BLUE]getpoint[/color][/b] [b][color=#ff00ff]"\nSELECT PLINE FOR CAREPET AREA: "[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
          [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] p5 [b][color=RED]([/color][/b][b][color=BLUE]getpoint[/color][/b] [b][color=#ff00ff]"\nWHERE TO PLACE TEXT: "[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]           
   
   [i][color=#990099];; Allow for a null input, so use an IF function.[/color][/i]
   [i][color=#990099];; I have grouped both inputs here, so that both can be[/color][/i]
   [i][color=#990099];; tested for before proceeding.[/color][/i]

   [b][color=RED]([/color][/b][b][color=BLUE]progn[/color][/b]
     [i][color=#990099];; Wrap the following code so that it is evaluated as[/color][/i]
     [i][color=#990099];; one expression. As the IF function only takes a "test"[/color][/i]
     [i][color=#990099];; expression, "then" expression & "else" expression (optional),[/color][/i]
     [i][color=#990099];; We want to wrap all the following code into the "then" expression.[/color][/i]
     
     [i][color=#990099];; (setvar "osmode" 0)  ;; <<-- Change this AFTER the user has selected the points[/color][/i]
     
     [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] p5 [b][color=RED]([/color][/b][b][color=BLUE]polar[/color][/b] p5 [b][color=BLUE]pi[/color][/b] [b][color=#009900]1250[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] p6 [b][color=RED]([/color][/b][b][color=BLUE]polar[/color][/b] p5 [b][color=#009900]0[/color][/b] [b][color=#009900]2500[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] fn [b][color=RED]([/color][/b][b][color=BLUE]getstring[/color][/b] [b][color=BLUE]t[/color][/b] [b][color=#ff00ff]"\nFLAT NO.:"[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     [b][color=RED]([/color][/b][b][color=BLUE]command[/color][/b] [b][color=#ff00ff]"AREA"[/color][/b] [b][color=#ff00ff]"E"[/color][/b] P1[b][color=RED])[/color][/b]
     [b][color=RED]([/color][/b][b][color=BLUE]SETQ[/color][/b] A [b][color=RED]([/color][/b][b][color=BLUE]GETVAR[/color][/b] [b][color=#ff00ff]"AREA"[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     
     [i][color=#990099];; (SETQ F (SF)) ;; <<-- I'm not sure what this is trying to achieve.[/color][/i]
     
     [b][color=RED]([/color][/b][b][color=BLUE]SETQ[/color][/b] B [b][color=RED]([/color][/b][b][color=BLUE]/[/color][/b] A [b][color=#009900]1000000[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     [b][color=RED]([/color][/b][b][color=BLUE]SETQ[/color][/b] C [b][color=RED]([/color][/b][b][color=BLUE]RTOS[/color][/b] B [b][color=#009900]2[/color][/b] [b][color=#009900]2[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     
     [b][color=RED]([/color][/b][b][color=BLUE]SETQ[/color][/b] D [b][color=#ff00ff]"CARPET AREA OF FLAT"[/color][/b][b][color=RED])[/color][/b] [i][color=#990099];; No need for the strcat, only one string here[/color][/i]
     
     [b][color=RED]([/color][/b][b][color=BLUE]setq[/color][/b] e [b][color=RED]([/color][/b][b][color=BLUE]strcat[/color][/b] [b][color=#ff00ff]"NO. "[/color][/b] FN [b][color=#ff00ff]" = "[/color][/b] C [b][color=#ff00ff]" SQ.MT."[/color][/b][b][color=RED])[/color][/b][b][color=RED])[/color][/b]
     
     [i][color=#990099];; (setq F (strcat "       = " F)) ;; See "F" above.[/color][/i]
     
     [b][color=RED]([/color][/b][b][color=BLUE]COMMAND[/color][/b] [b][color=#ff00ff]"TEXT"[/color][/b] [b][color=#ff00ff]"S"[/color][/b] [b][color=#ff00ff]"STANDARD"[/color][/b] [b][color=#ff00ff]"f"[/color][/b] P5 p6 [b][color=#ff00ff]"275"[/color][/b] D [b][color=#ff00ff]"text"[/color][/b] [b][color=#ff00ff]""[/color][/b] e
              
              [i][color=#990099];"text" "" f ;; << see "f" above.[/color][/i]
              
              [b][color=RED])[/color][/b] [i][color=#990099]; End command[/color][/i]

   [b][color=RED])[/color][/b] [i][color=#990099];; End Progn[/color][/i]

   [b][color=RED]([/color][/b][b][color=BLUE]princ[/color][/b] [b][color=#ff00ff]"\n<< No Points Selected >>"[/color][/b][b][color=RED])[/color][/b] [i][color=#990099];; Else no Points were selected.[/color][/i]

 [b][color=RED])[/color][/b]  [i][color=#990099];; End IF[/color][/i]

 [i][color=#990099];; Reset System Variables:[/color][/i]
     
 [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"cmdecho"[/color][/b] oldCM[b][color=RED])[/color][/b]
 [b][color=RED]([/color][/b][b][color=BLUE]setvar[/color][/b] [b][color=#ff00ff]"osmode"[/color][/b]  oldos[b][color=RED])[/color][/b]

 [b][color=RED]([/color][/b][b][color=BLUE]princ[/color][/b][b][color=RED])[/color][/b] [i][color=#990099];; Exit Cleanly[/color][/i]

[b][color=RED])[/color][/b] [i][color=#990099];; End defun[/color][/i]

Overall, the program is pretty good for a first start.

A few pointers to help you:

• Use more descriptive names for the variable you use, instead of single letters, it will make the code easier to read.
  
• Use "princ" at the end of the program to exit cleanly.
  
• Reset ALL sys vars to the original values, not just what you think they should be.

Lee

Dear Sir, (Freerefill & lee mac)

thnx sir u r comments

ur great..............

thnxxxxxxxxxxxxx

Hi,

While the time i send the drawings by email i have to attach the ctb also.is it possibile to bind the ctb within the drawing.and if we did so how it works?

Please advise.

Take note of my comments Sachindkini,

 

[b][color=red]([/color][/b][b][color=blue]defun[/color][/b] c:CA2  [b][color=red]([/color][/b][b][color=blue]/[/color][/b] *error* p1 p5 p6 a b c d e fn[b][color=red])[/color][/b]

 [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] oldCM [b][color=red]([/color][/b][b][color=blue]getvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b][b][color=red])[/color][/b]
       oldos [b][color=red]([/color][/b][b][color=blue]getvar[/color][/b] [b][color=#ff00ff]"OSMODE"[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]

 [i][color=#990099];; Store the original values of the[/color][/i]
 [i][color=#990099];; system variables before you change them,[/color][/i]
 [i][color=#990099];; then you can reset them at the end.[/color][/i]
 [i][color=#990099];; I see you did this for the OSMODE, but it[/color][/i]
 [i][color=#990099];; is good practice to do it will all sys vars.[/color][/i]

 [b][color=red]([/color][/b][b][color=blue]defun[/color][/b] *error* [b][color=red]([/color][/b]msg[b][color=red])[/color][/b]
   [b][color=red]([/color][/b][b][color=blue]if[/color][/b] oldCM [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b] oldCM[b][color=red])[/color][/b][b][color=red])[/color][/b]
   [b][color=red]([/color][/b][b][color=blue]if[/color][/b] oldos [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"OSMODE"[/color][/b] oldos[b][color=red])[/color][/b][b][color=red])[/color][/b]
   [b][color=red]([/color][/b][b][color=blue]princ[/color][/b] msg[b][color=red])[/color][/b]
   [b][color=red]([/color][/b][b][color=blue]princ[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]

 [i][color=#990099];; Include a short error handler to reset the[/color][/i]
 [i][color=#990099];; System Variables, should the user hit Esc[/color][/i]
 [i][color=#990099];; during the program. Remember to localise this[/color][/i]
 [i][color=#990099];; function, as it is defined within the main function.[/color][/i]

 [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"CMDECHO"[/color][/b] [b][color=#009900]0[/color][/b][b][color=red])[/color][/b]
 [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"osmode"[/color][/b] [b][color=#009900]524[/color][/b][b][color=red])[/color][/b]

 [b][color=red]([/color][/b][b][color=blue]if[/color][/b] [b][color=red]([/color][/b][b][color=blue]and[/color][/b] [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] p1 [b][color=red]([/color][/b][b][color=blue]getpoint[/color][/b] [b][color=#ff00ff]"\nSELECT PLINE FOR CAREPET AREA: "[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]
          [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] p5 [b][color=red]([/color][/b][b][color=blue]getpoint[/color][/b] [b][color=#ff00ff]"\nWHERE TO PLACE TEXT: "[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]           

   [i][color=#990099];; Allow for a null input, so use an IF function.[/color][/i]
   [i][color=#990099];; I have grouped both inputs here, so that both can be[/color][/i]
   [i][color=#990099];; tested for before proceeding.[/color][/i]

   [b][color=red]([/color][/b][b][color=blue]progn[/color][/b]
     [i][color=#990099];; Wrap the following code so that it is evaluated as[/color][/i]
     [i][color=#990099];; one expression. As the IF function only takes a "test"[/color][/i]
     [i][color=#990099];; expression, "then" expression & "else" expression (optional),[/color][/i]
     [i][color=#990099];; We want to wrap all the following code into the "then" expression.[/color][/i]

     [i][color=#990099];; (setvar "osmode" 0)  ;; <<-- Change this AFTER the user has selected the points[/color][/i]

     [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] p5 [b][color=red]([/color][/b][b][color=blue]polar[/color][/b] p5 [b][color=blue]pi[/color][/b] [b][color=#009900]1250[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]
     [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] p6 [b][color=red]([/color][/b][b][color=blue]polar[/color][/b] p5 [b][color=#009900]0[/color][/b] [b][color=#009900]2500[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]
     [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] fn [b][color=red]([/color][/b][b][color=blue]getstring[/color][/b] [b][color=blue]t[/color][/b] [b][color=#ff00ff]"\nFLAT NO.:"[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]
     [b][color=red]([/color][/b][b][color=blue]command[/color][/b] [b][color=#ff00ff]"AREA"[/color][/b] [b][color=#ff00ff]"E"[/color][/b] P1[b][color=red])[/color][/b]
     [b][color=red]([/color][/b][b][color=blue]SETQ[/color][/b] A [b][color=red]([/color][/b][b][color=blue]GETVAR[/color][/b] [b][color=#ff00ff]"AREA"[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]

     [i][color=#990099];; (SETQ F (SF)) ;; <<-- I'm not sure what this is trying to achieve.[/color][/i]

     [b][color=red]([/color][/b][b][color=blue]SETQ[/color][/b] B [b][color=red]([/color][/b][b][color=blue]/[/color][/b] A [b][color=#009900]1000000[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]
     [b][color=red]([/color][/b][b][color=blue]SETQ[/color][/b] C [b][color=red]([/color][/b][b][color=blue]RTOS[/color][/b] B [b][color=#009900]2[/color][/b] [b][color=#009900]2[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]

     [b][color=red]([/color][/b][b][color=blue]SETQ[/color][/b] D [b][color=#ff00ff]"CARPET AREA OF FLAT"[/color][/b][b][color=red])[/color][/b] [i][color=#990099];; No need for the strcat, only one string here[/color][/i]

     [b][color=red]([/color][/b][b][color=blue]setq[/color][/b] e [b][color=red]([/color][/b][b][color=blue]strcat[/color][/b] [b][color=#ff00ff]"NO. "[/color][/b] FN [b][color=#ff00ff]" = "[/color][/b] C [b][color=#ff00ff]" SQ.MT."[/color][/b][b][color=red])[/color][/b][b][color=red])[/color][/b]

     [i][color=#990099];; (setq F (strcat "       = " F)) ;; See "F" above.[/color][/i]

     [b][color=red]([/color][/b][b][color=blue]COMMAND[/color][/b] [b][color=#ff00ff]"TEXT"[/color][/b] [b][color=#ff00ff]"S"[/color][/b] [b][color=#ff00ff]"STANDARD"[/color][/b] [b][color=#ff00ff]"f"[/color][/b] P5 p6 [b][color=#ff00ff]"275"[/color][/b] D [b][color=#ff00ff]"text"[/color][/b] [b][color=#ff00ff]""[/color][/b] e

              [i][color=#990099];"text" "" f ;; << see "f" above.[/color][/i]

              [b][color=red])[/color][/b] [i][color=#990099]; End command[/color][/i]

   [b][color=red])[/color][/b] [i][color=#990099];; End Progn[/color][/i]

   [b][color=red]([/color][/b][b][color=blue]princ[/color][/b] [b][color=#ff00ff]"\n<< No Points Selected >>"[/color][/b][b][color=red])[/color][/b] [i][color=#990099];; Else no Points were selected.[/color][/i]

 [b][color=red])[/color][/b]  [i][color=#990099];; End IF[/color][/i]

 [i][color=#990099];; Reset System Variables:[/color][/i]

 [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"cmdecho"[/color][/b] oldCM[b][color=red])[/color][/b]
 [b][color=red]([/color][/b][b][color=blue]setvar[/color][/b] [b][color=#ff00ff]"osmode"[/color][/b]  oldos[b][color=red])[/color][/b]

 [b][color=red]([/color][/b][b][color=blue]princ[/color][/b][b][color=red])[/color][/b] [i][color=#990099];; Exit Cleanly[/color][/i]

[b][color=red])[/color][/b] [i][color=#990099];; End defun[/color][/i]

Overall, the program is pretty good for a first start.

 

 

 

A few pointers to help you:

• Use more descriptive names for the variable you use, instead of single letters, it will make the code easier to read.
  
• Use "princ" at the end of the program to exit cleanly.
  
• Reset ALL sys vars to the original values, not just what you think they should be.

Lee

;Dear Sir,

;Add some comment plz chk sir

;how to create pline frame around the text

(defun c:CA3 (/ *error* p1 p5 p6 a b c d e fn)

(setq oldCM (getvar "CMDECHO")

oldos (getvar "OSMODE"))

;; Store the original values of the

;; system variables before you change them,

;; then you can reset them at the end.

;; I see you did this for the OSMODE, but it

;; is good practice to do it will all sys vars.

(defun *error* (msg)

(if oldCM (setvar "CMDECHO" oldCM))

(if oldos (setvar "OSMODE" oldos))

(princ msg)

(princ))

;; Include a short error handler to reset the

;; System Variables, should the user hit Esc

;; during the program. Remember to localise this

;; function, as it is defined within the main function.

(setvar "CMDECHO" 0)

(setvar "osmode" 524)

(if (and (setq p1 (getpoint "\nSELECT PLINE FOR CAREPET AREA: "))

(setq p5 (getpoint "\nWHERE TO PLACE TEXT: ")))

;; Allow for a null input, so use an IF function.

;; I have grouped both inputs here, so that both can be

;; tested for before proceeding.

(progn

;; Wrap the following code so that it is evaluated as

;; one expression. As the IF function only takes a "test"

;; expression, "then" expression & "else" expression (optional),

;; We want to wrap all the following code into the "then" expression.

;; (setvar "osmode" 0) ;;

(setq p5 (polar p5 pi 1250))

(setq p6 (polar p5 0 2500))

(setq fn (getstring t "\nFLAT NO.:"))

(command "AREA" "E" P1)

(SETQ A (GETVAR "AREA"))

;;

(SETQ F (SF)); area convert to sq.ft.

(defun sf (/ sm sf st)

(setq sm (getvar "area"))

(setq sf (* sm 0.000010764))

(setq sf (rtos sf 2 2))

(SETQ ST (strcat sf " SQ. FT."))

(PRIN1 ST)

)

(SETQ B (/ A 1000000))

(SETQ C (RTOS B 2 2))

(SETQ D "CARPET AREA OF FLAT") ;; No need for the strcat, only one string here

(setq e (strcat "NO. " FN " = " C " SQ.MT."))

(setq F (strcat " = " F))

(COMMAND "TEXT" "S" "STANDARD" "f" P5 p6 "275" D "text" "" e

"text" "" f

) ; End command

) ;; End Progn

(princ "\n>") ;; Else no Points were selected.

) ;; End IF

;; Reset System Variables:

(setvar "cmdecho" oldCM)

(setvar "osmode" oldos)

(princ) ;; Exit Cleanly

) ;; End defun

Is this a separate question?

Hello Sir,

Yes It's a seperate question.please advise. thank you

Shibu

Hello Sir,

 

Yes It's a seperate question.please advise. thank you

 

Shibu

I am not sure, but with a new question, please search for the topic first, then, if nothing is found, create a new thread in the relevant forum.

I'm sure a Moderator will move your off-topic posts for you.