Author Topic: Constructive Critism?  (Read 14986 times)

0 Members and 1 Guest are viewing this topic.

ronjonp

  • Needs a day job
  • Posts: 7526
Constructive Critism?
« on: October 28, 2003, 11:33:15 AM »
Any criticism on how this is written?

Code: [Select]
;Inserts barscale and prompts for scale.

(defun C:barscale (/ cmd att clay bsscale inpt)
  (setq cmd (getvar "cmdecho"))
  (setvar "cmdecho" 0)
  (setq att (getvar "attdia"))
  (setvar "attdia" 0)
  (setq clay (getvar "clayer"))
  (command ".-layer" "m" "M-SHEET" "" "")
  (setq bsscale (getreal "\nEnter Scale: "))
  (setq inpt (getpoint "\nPick barscale location"))
  (command ".-INSERT"
  "R:\\AEITITLE\\Barscale.dwg"
  inpt
  "1"
  "1"
  "0"
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 0) 2 0) "'")
  (strcat (rtos (/ bsscale 2) 2 0) "'")
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 2) 2 0) "'")
  )
  (setvar "cmdecho" cmd)
  (setvar "attdia" att)
  (setvar "clayer" clay)
  (princ)
)



Thanks,

Ron :D


7 Edit: I added code tags to your post.

Windows 11 x64 - AutoCAD /C3D 2023

Custom Build PC

JohnK

  • Administrator
  • Seagull
  • Posts: 10605
Constructive Critism?
« Reply #1 on: October 28, 2003, 11:45:35 AM »
w00t! I like it. Its orginized, very nice. You get only varibles you have to change and you localized all the variables you created. Very nice.

Now if i were you i would add some checks and tests to see if the enviroment is safe before you actualy change or do anything it would be a great little program. Ill whip up a simple example that you can get an idea as to what im talking about.
TheSwamp.org (serving the CAD community since 2003)
Member location map - Add yourself

Donate to TheSwamp.org

JohnK

  • Administrator
  • Seagull
  • Posts: 10605
Constructive Critism?
« Reply #2 on: October 28, 2003, 12:04:04 PM »
Take a look at these lines of code:
Code: [Select]
;; loop untill the test expression evals to nil
(while
  ;; Not will return TRUE if the test evaluates to nil; (start of test expression)
  (not
    ;; Our getpoint function will return nil if there isnt a point
    ;; selected; (the root test expression)
    (setq pt (getpoint))
    );_ end not
  );_ end while


Now copy this code into your command line (or the VLIDE) and run it. Try to right click instead of actualy picking a point. what happens?

Do you see what I am doing? I am running a couple of tests to see if the end user actualy selected a point. If they didnt, then do it again, and keep asking for a point untill they get it right.

We can even make a neat little function out of this concept. Lets do that.
Code: [Select]
(defun RealyGetPoint (/ pt)
(while
  (not (setq pt (getpoint "\nPlease select a point: ")))
  (princ "\nPlease try again..."))
pt
)


This is a simple concept that you should be able to grasp and apply it to the rest of your code. -i.e. test to see if the "environment" is safe before you start to change, get, modify, or do anything. A few "if" statments, and some extra thought, should get you a very nice program with prompts to the end user if there happens to be a problem.

Think of areas in this program that could crash. (What has to be present in order for this program to run?)
TheSwamp.org (serving the CAD community since 2003)
Member location map - Add yourself

Donate to TheSwamp.org

ronjonp

  • Needs a day job
  • Posts: 7526
Constructive Critism?
« Reply #3 on: October 28, 2003, 12:05:03 PM »
Thanks Se7en  :D .

When I run it it works fine but it prompts this on the command line before it starts.

Command: barscale
Unknown command "BARSCALE".  Press F1 for help.

Enter Scale:

Any suggestions on how to suppress this?

Windows 11 x64 - AutoCAD /C3D 2023

Custom Build PC

SMadsen

  • Guest
Constructive Critism?
« Reply #4 on: October 28, 2003, 12:34:53 PM »
ronjonp, remove the last return in the LAYER command:
(command ".-layer" "m" "M-SHEET" "" "")
->
(command ".-layer" "m" "M-SHEET" "")

daron

  • Guest
Constructive Critism?
« Reply #5 on: October 28, 2003, 01:41:51 PM »
Wouldn't (initget 0) be just as effective before (getpoint ...) and simpler than writing out a (while (not...)) function? Not to take from your lesson 7, but just something to think about. As long as I'm in this subject: Ron, think about this, lisp will process many things behind the scenes. (setq's ...) can all be lumped together under one setq, as long as it isn't needed after another function or subroutine, i.e.
Code: [Select]
(setq cmd (getvar "cmdecho")
        att (getvar 'attdia)
        clay (getvar 'clayer)
)
(setvar "cmdecho" 0)
(setvar "attdia" 0)

Then, consider using
Code: [Select]
(initget 7) ;check the values in the help for your preference
(setq bsscale (getreal "\nEnter Scale: "))
(initget 1) ;same as above applies here. Edited here. Thinkin bit.
(setq inpt (getpoint "\nPick barscale location: "))

At this point you could create your layer and group both commands into one. It all depends on personal preference, though.
Code: [Select]
(command ".-layer" "m" "M-SHEET" ""
                ".-insert" "R:\\AEITITLE\\Barscale.dwg"
                inpt
                "1"
                "1"
                "0"
      (strcat (rtos bsscale 2 0) "'")
      (strcat (rtos (* bsscale 0) 2 0) "'")
      (strcat (rtos (/ bsscale 2) 2 0) "'")
      (strcat (rtos bsscale 2 0) "'")
      (strcat (rtos (* bsscale 2) 2 0) "'")
  )
  (setvar 'cmdecho cmd)
  (setvar "attdia" att)
  (setvar "clayer" clay)
  (princ)

You'll notice that you can't lump setvar like you can command and setq. You'll also notice that in getvar and setvar you can call the variable in double quotes or preceded by a single quote. Last, take a look at leaving out an option that'll wait for user input and see what happens to the resetting of your variables.

ronjonp

  • Needs a day job
  • Posts: 7526
Constructive Critism?
« Reply #6 on: October 28, 2003, 01:55:55 PM »
Thanks guys. Here is what I have now.

Code: [Select]
(defun C:barscale (/ scl cmd att clay bsscale pt)
  (setq scl (getvar "dimscale"))
  (setq cmd (getvar "cmdecho"))
  (setvar "cmdecho" 0)
  (setq att (getvar "attdia"))
  (setvar "attdia" 0)
  (setq clay (getvar "clayer"))
  (command ".-layer" "m" "M-SHEET" "")
  (setq bsscale (getreal "\nEnter Scale: "))
  (while
    (not
      (setq pt (getpoint "\nPick barscale location"))
    )
  )
  (command ".-INSERT"
  "R:\\AEITITLE\\Barscale.dwg"
  pt
  scl
  scl
  "0"
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 0) 2 0) "'")
  (strcat (rtos (/ bsscale 2) 2 0) "'")
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 2) 2 0) "'")
  )
  (setvar "cmdecho" cmd)
  (setvar "attdia" att)
  (setvar "clayer" clay)
  (princ)
)
(C:barscale)


Hey Seven,

How do I apply that function?

Windows 11 x64 - AutoCAD /C3D 2023

Custom Build PC

JohnK

  • Administrator
  • Seagull
  • Posts: 10605
Constructive Critism?
« Reply #7 on: October 28, 2003, 02:38:43 PM »
Like this: "(RealyGetPoint)"

But do you understand what the function is doing and how the general concept behind that function can be used in your program?
The function isnt all that special, i was trying to teach/demonstrate to you something with it.
TheSwamp.org (serving the CAD community since 2003)
Member location map - Add yourself

Donate to TheSwamp.org

ronjonp

  • Needs a day job
  • Posts: 7526
Constructive Critism?
« Reply #8 on: October 28, 2003, 05:47:22 PM »
Daron,

I like the idea of grouping things better. I didn't even think of putting the make layer and insert together and the variables are easier to see when all together. What is the advantage of initget over se7en's suggestion?
Thank you all for your help.

Ron

Here is my end product:

Code: [Select]
(defun C:barscale (/ scl cmd att clay bsscale pt)
  (setq scl  (getvar 'dimscale)
cmd  (getvar 'cmdecho)
att  (getvar 'attdia)
clay (getvar 'clayer)
  )
  (setvar 'cmdecho 0)
  (setvar 'attdia 0)
  (initget 7)
  (setq bsscale (getreal "\nEnter Scale: "))
  (initget 7)
  (setq pt (getpoint "\nPick barscale location"))
  (command ".-layer"
  "m"
  "M-SHEET"
  ""
  ".-INSERT"
  "R:\\AEITITLE\\Barscale.dwg"
  pt
  scl
  scl
  "0"
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 0) 2 0) "'")
  (strcat (rtos (/ bsscale 2) 2 0) "'")
  (strcat (rtos bsscale 2 0) "'")
  (strcat (rtos (* bsscale 2) 2 0) "'")
  )
  (setvar "cmdecho" cmd)
  (setvar "attdia" att)
  (setvar "clayer" clay)
  (princ)
)
(C:barscale)

Windows 11 x64 - AutoCAD /C3D 2023

Custom Build PC

daron

  • Guest
Constructive Critism?
« Reply #9 on: October 28, 2003, 06:10:37 PM »
Read the help file on initget. It appears to me that you can't use a 7 on the getpoint and it wouldn't make sense to do so. Haven't tested it though. One advantage to this over se7en's suggestion, would be that it's shorter code, easier to maintain and less likelyhood of errors. It's a preference really. I use the (while (not ...)) loop mostly for entsel, nentsel, or nentselp, since those are the only one's that don't honor bit code values. Either way would work.

JohnK

  • Administrator
  • Seagull
  • Posts: 10605
Constructive Critism?
« Reply #10 on: October 28, 2003, 08:07:37 PM »
*sigh*

Im trying to teach you error trapping. Take the loop i gave you as an example and study it. what does each line return? What does each line mean? How can you use the concept of my example in your program to improve it? I didnt give you any code you couldnt find anywhere else. (I was trying to teach you.)

Now learn damn you, learn!

:P

(let me know when you understand how the loop works.)
TheSwamp.org (serving the CAD community since 2003)
Member location map - Add yourself

Donate to TheSwamp.org

ronjonp

  • Needs a day job
  • Posts: 7526
Constructive Critism?
« Reply #11 on: October 29, 2003, 08:54:15 AM »
Hey Seven,

I have learned from all of these posts. I'm just a slow learner :D (when it comes to lisp). Bear with me and I'll get there.

-Ron

Windows 11 x64 - AutoCAD /C3D 2023

Custom Build PC

Mark

  • Custom Title
  • Seagull
  • Posts: 28753
Re: Constructive Critism?
« Reply #12 on: October 29, 2003, 09:05:50 AM »
Quote from: ronjonp
Any criticism on how this is written?

Only if you plan on giving the program to others, if not I think it will get the job done. I have one similar, it has 0 error checking in it! That's because I'm the only one using it.
TheSwamp.org  (serving the CAD community since 2003)

nivuahc

  • Guest
Constructive Critism?
« Reply #13 on: October 29, 2003, 10:07:16 AM »
I'm sure you can tweak this to suit your needs...

Code: [Select]
(DEFUN MAKEBS ()
(ENTMAKE '((0 . "BLOCK")(2 . "BARSCALE")(8 . "0") (70 . 66)(10 0.0 0.0 0.0)))

(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 0.715717 -22.7268 0.0) (40 . 12.0) (1 . "0")     (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 22.5262 -22.7268 0.0)  (40 . 12.0) (1 . "2'")    (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 48.6081 -22.7268 0.0)  (40 . 12.0) (1 . "4'")    (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 95.9303 -22.7268 0.0)  (40 . 12.0) (1 . "8'")    (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 191.507 -22.7268 0.0)  (40 . 12.0) (1 . "16'")   (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 279.661 -22.7268 0.0)  (40 . 12.0) (1 . "24'")   (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))
(ENTMAKE '((0 . "TEXT")  (8 . "0") (62 . 0) (10 318.56 0.36234 0.0)    (40 . 12.0) (1 . "SCALE") (41 . 1.0) (7 . "STANDARD") (72 . 0) (11 0.0 0.0 0.0) (73 . 0)))

(ENTMAKE '((0 . "ATTDEF") (8 . "0") (10 318.56 -19.6377 0.0) (40 . 12.0) (1 . "") (41 . 1.0) (7 . "STANDARD") (72 . 0) (70 . 0) (11 0.0 0.0 0.0) (3 . "Scale Factor") (2 . "scale")))

(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 291.721 2.06301 0.0)  (11 291.721 11.063 0.0)  (12 195.721 2.06301 0.0)  (13 195.721 11.063 0.0)  (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 99.7205 2.06301 0.0)  (11 99.7205 11.063 0.0)  (12 51.7205 2.06301 0.0)  (13 51.7205 11.063 0.0)  (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 15.7205 -6.93699 0.0) (11 15.7205 2.06301 0.0) (12 9.72052 -6.93699 0.0) (13 9.72052 2.06301 0.0) (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 9.72052 2.06301 0.0)  (11 9.72052 11.063 0.0)  (12 3.72052 2.06301 0.0)  (13 3.72052 11.063 0.0)  (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 27.7205 2.06301 0.0)  (11 27.7205 11.063 0.0)  (12 15.7205 2.06301 0.0)  (13 15.7205 11.063 0.0)  (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 51.7205 -6.93699 0.0) (11 51.7205 2.06301 0.0) (12 27.7205 -6.93699 0.0) (13 27.7205 2.06301 0.0) (39 . 0.0)))
(ENTMAKE '((0 . "TRACE")  (8 . "0") (62 . 0) (10 195.721 -6.93699 0.0) (11 195.721 2.06301 0.0) (12 99.7205 -6.93699 0.0) (13 99.7205 2.06301 0.0) (39 . 0.0)))

(ENTMAKE '((0 . "LINE")  (8 . "0") (62 . 0) (10 3.72052 3.72864 0.0) (11 3.72052 -6.93699 0.0)))
(ENTMAKE '((0 . "LINE")  (8 . "0") (62 . 0) (10 291.721 11.063 0.0)  (11 291.721 -6.93699 0.0)))

(ENTMAKE '((0 . "ENDBLK")))
)



 :P

JohnK

  • Administrator
  • Seagull
  • Posts: 10605
Constructive Critism?
« Reply #14 on: October 29, 2003, 10:48:39 AM »
Quote from: ronjonp
Hey Seven,
I have learned from all of these posts. I'm just a slow learner :D (when it comes to lisp). Bear with me and I'll get there.

-Ron

Good, I hope they help clear some things up and help you create a beter program. Let us know what you dont understand so we can help you out.

Ive said before, that I try not to just give you the answer with code, I try to hide answers in the code I give you. In the example I gave you, I was trying to tell you that you need to test the results from several areas in your code before heading off and modifying values. You got a good base there, but...
Quote from: Mark Thomas
Only if you plan on giving the program to others, if not I think it will get the job done. I have one similar, it has 0 error checking in it! That's because I'm the only one using it.


My thoughts exactly.
TheSwamp.org (serving the CAD community since 2003)
Member location map - Add yourself

Donate to TheSwamp.org

SMadsen

  • Guest
Constructive Critism?
« Reply #15 on: October 29, 2003, 12:45:28 PM »
nivuach,
Ever heard of MAPCAR? :D

.. and nested defun's? (http://theswamp.org/phpBB2/viewtopic.php?t=161)

Code: [Select]
(defun MAKEBS (/ entmakeText entmakeTrace)
  (defun entmakeText (lst / ins hgt txt)
    (mapcar 'set '(ins hgt txt) lst)
    (entmake (append '((0 . "TEXT") (8 . "0")(7 . "Standard"))
                     (list (cons 10 ins) (cons 40 hgt) (cons 1 txt))))
  )
  (defun entmakeTrace (lst / p1 p2 p3 p4)
    (mapcar 'set '(p1 p2 p3 p4) lst)
    (entmake (append '((0 . "TRACE") (8 . "0"))
                     (list (cons 10 p1) (cons 11 p2) (cons 12 p3) (cons 13 p4))))
  )
  (entmake '((0 . "BLOCK") (2 . "BARSCALE") (8 . "0") (70 . 66) (10 0.0 0.0 0.0)))
  (mapcar 'entmakeText
          '(((0.715717 -22.7268 0.0) 12.0 "0")
            ((22.5262 -22.7268 0.0) 12.0 "2'")
            ((48.6081 -22.7268 0.0) 12.0 "4'")
            ((95.9303 -22.7268 0.0) 12.0 "8'")
            ((191.507 -22.7268 0.0) 12.0 "16'")
            ((279.661 -22.7268 0.0) 12.0 "24'")
            ((318.56 0.36234 0.0) 12.0 "SCALE")
           )
  )
  (entmake '((0 . "ATTDEF")(8 . "0")(10 318.56 -19.6377 0.0)(40 . 12.0)(70 . 0) (1 . "")
             (7 . "Standard")(3 . "Scale Factor")(2 . "scale"))
  )
  (mapcar 'entmakeTrace
          '(((291.721 2.06301 0.0)(291.721 11.063 0.0)
             (195.721 2.06301 0.0)(195.721 11.063 0.0))
            ((99.7205 2.06301 0.0)(99.7205 11.063 0.0)
             (51.7205 2.06301 0.0)(51.7205 11.063 0.0))
            ((15.7205 -6.93699 0.0)(15.7205 2.06301 0.0)
             (9.72052 -6.93699 0.0)(9.72052 2.06301 0.0))
            ((9.72052 2.06301 0.0)(9.72052 11.063 0.0)
             (3.72052 2.06301 0.0)(3.72052 11.063 0.0))
            ((27.7205 2.06301 0.0)(27.7205 11.063 0.0)
             (15.7205 2.06301 0.0)(15.7205 11.063 0.0))
            ((51.7205 -6.93699 0.0)(51.7205 2.06301 0.0)
             (27.7205 -6.93699 0.0)(27.7205 2.06301 0.0))
            ((195.721 -6.93699 0.0)(195.721 2.06301 0.0)
             (99.7205 -6.93699 0.0)(99.7205 2.06301 0.0))
           )
  )
  (entmake
    '((0 . "LINE") (8 . "0") (62 . 0) (10 3.72052 3.72864 0.0) (11 3.72052 -6.93699 0.0)))
  (entmake
    '((0 . "LINE") (8 . "0") (62 . 0) (10 291.721 11.063 0.0) (11 291.721 -6.93699 0.0)))
  (entmake '((0 . "ENDBLK")(100 . "AcDbBlockEnd")))
)

daron

  • Guest
Constructive Critism?
« Reply #16 on: October 29, 2003, 02:11:14 PM »
Ahh! I knew there was a way to make entmake stop hurting my eyes. I've avoided entmake in the past because I've only seen people use it the way Chuck did. Stig, that is so much nicer. Great idea. I've wondered also, why entmake is so popular when inserting a block is so much easier. What makes writing all that entmake code so desirable?

SMadsen

  • Guest
Constructive Critism?
« Reply #17 on: October 29, 2003, 03:44:58 PM »
Well, you know those guys and gals that sometimes uses your seat and think that spring cleaning is due every day? They delete what they don't understand .. including company blocks!

Ok, so I guess the main reason is for parametric design. Instead of having 5 barscale blocks predefined in some directory and more or less hardcoded paths in your lisp routines, you could offer a dialog with nice adjustable features.

This was just for fun, though. Judging from the file that nivuach links to in the signature, I'll bet nivuach has written a program that converts a drawing to ENTMAKE calls :)

I would use wblocks any time, also (if no parametric design is needed). But inserting the block I would probably do with ENTMAKE. It's fast, it's clean and it can be tested more easily than a command (unless VL-CMDF is used).

daron

  • Guest
Constructive Critism?
« Reply #18 on: October 30, 2003, 10:33:50 AM »
Moved the race to its own thread. RJP, back to your regularly scheduled program.

nivuahc

  • Guest
Constructive Critism?
« Reply #19 on: October 31, 2003, 09:32:52 AM »
Quote
Ever heard of MAPCAR?  

.. and nested defun's?


C'mon... I threw that together in about 5 minutes!  :shock:

:P

SMadsen

  • Guest
Constructive Critism?
« Reply #20 on: October 31, 2003, 11:00:47 AM »
It was just for fun, nivuahc

Btw, here's a thing that can make entmakes of almost anything pulled from a drawing:
Code: [Select]
(defun C:makeEntmake (/ a dwg path ent entl fn sset)
  (setq path (getvar "DWGPREFIX")
        dwg  (strcat path (vl-filename-base (getvar "DWGNAME")) ".lsp")
        fn   (open dwg "w")
        a    0
  )
  (cond
    (fn
     (cond ((setq sset (ssget))
            (repeat (sslength sset)
              (setq ent  (ssname sset a)
                    entl (entget ent)
              )
              (mapcar (function (lambda (n) (setq entl (vl-remove (assoc n entl) entl))))
                      '(-2 -1 5 102 300 330 331 350 360)
              )
              (write-line (strcat "(entmake '" (vl-prin1-to-string entl) ")") fn)
              (setq a (1+ a))
            )
           )
     )
     (close fn)
    )
  )
  (princ)
)

Kerry

  • Mesozoic relic
  • Seagull
  • Posts: 11654
  • class keyThumper<T>:ILazy<T>
Constructive Critism?
« Reply #21 on: October 31, 2003, 07:27:19 PM »
Makes my old heart sing.
kdub, kdub_nz in other timelines.
Perfection is not optional.
Everything will work just as you expect it to, unless your expectations are incorrect.
Discipline: None at all.