5
\$\begingroup\$

I am doing a CS practice exercise in which you are supposed to sort exactly three numbers using only if statements. I wrote the following code in LISP and would appreciate any suggestions how to improve on my code. Thank you!

(defun generate-three ()
  (loop for l from 1 to 3 collect (random 25)))

(defun result (a b c) (format t "The sorted result is (~d,~d,~d)~%" a b c))

(let (a b c (the-three (generate-three)))
    (format t "~a ~%" the-three)
    (setq a (first the-three) b (second the-three) c (third the-three))
    (if (< a b)
      (if (< b c) (result a b c)
        (if (< a c) (result a c b) (result c a b)))
      (if (< a c) (result b a c)
        (if (< b c) (result b c a) (result c b a)))))

Based on feedback from commenters, the revised code looks as follows:

(defun my-sort-3 (a b c)
    (if (< a b)
      (if (< b c) (list a b c)
        (if (< a c) (list a c b) (list c a b)))
      (if (< a c) (list b a c)
        (if (< b c) (list b c a) (list c b a)))))

(destructuring-bind (a b c) (loop repeat 3 collect (random 25))
  (format t "The sorted result of (~d,~d,~d) is ~a ~%" a b c (my-sort-3 a b c)))

Latest iteration - note the changes in the final "format" statement:

(defun my-sort-3 (a b c)
    (if (< a b)
      (if (< b c) (list a b c)
        (if (< a c) (list a c b) (list c a b)))
      (if (< a c) (list b a c)
        (if (< b c) (list b c a) (list c b a)))))

(destructuring-bind (a b c) (loop repeat 3 collect (random 25))
  (format t "The sorted result of (~{~d~^, ~}) is (~{~d~^, ~})~%" (list a b c) (my-sort-3 a b c)))
\$\endgroup\$
2
\$\begingroup\$

In response to your follow-up. If you're really keen to keep the final formatting of the result "(~d, ~d, ~d)", you can still do that by formatting a list, you'd just have a slightly more complicated directive.

(format t "The sorted result is (~{~d~^, ~})~%" 
          (apply #'my-sort-3 (loop repeat 3 collect (random 25))))

In a format directive ~{ ~} denotes a loop. It expects a list, and applies the contained directive to each element. Within curlies in a format directive, ~^ denotes a joining string. Anything after it is repeated except on the last iteration. So, assuming the loop generated (1 2 3), the result of the above statement would be

The sorted result is (3, 2, 1)
NIL

For more information on format, check Seibels' chapter on format (in fact the entire book might be useful to you. (That wasn't the Amazon link, by the way, the entire Practical Common Lisp is available for free online)), and the CLHS section on formatted output.

If you're up for a challenge, after reading some of the above you should be able to replace this

(cond ((eq 'right triangle-type) (status-message "is a right triangle." a b c))
    ((eq 'equilateral triangle-type) (status-message "is an equilateral triangle." a b c))
    ((eq 'isoceles triangle-type) (status-message "is an isoceles triangle." a b c))
    ((eq 'triangle triangle-type) (status-message "is a non-right, non-isoceles, non-equilateral triangle." a b c))
    (t (status-message "is not a triangle." a b c)))

with a single format statement in your triangle assignment.

\$\endgroup\$
3
\$\begingroup\$

It's been a while since I've used lisp, so I can't really critique your code for style or any common lisp gotchas.

That said, the biggest thing I notice here is that the code for generating the 3 numbers, sorting them, and printing the result are all tightly coupled with each other. I would suggest defining a separate function that takes three numbers and returns them in sorted order, which can then be combined with your existing functions for generating three numbers and printing three numbers to give the desired output. The added benefit, of course, is that this sort function can later be used to sort any three numbers, not just the three in this bit of code.

\$\endgroup\$
2
\$\begingroup\$

Since you have to use ifs, I won't comment on that other than to say that cond (or, more likely, a recursive call) is probably the right choice here.


The loop you use in generate-three is slightly more complicated than you need it to be. You can specify what you need with (loop repeat 3 collect (random 25)). That's short enough that you won't need an intermediate function for this exercise. loop is a fairly complicated construct (probably the most complicated in Lisp). If you really want to come to grips with it, read through The CL Cookbook entry and Seibels' Loop chapter from Practical Common Lisp.


There's no need to declare variables in Lisp, so the a b c (effectively (a nil) (b nil) (c nil)) in the initial let statement is unnecessary. Since the only reason you're declaring those nil variables is to assign them later, this would be a good place to use destructuring-bind

(destructuring-bind (a b c) (loop repeat 3 collect (random 25))
  (format t "~a ~%" (list a b c))
  (if (< a b)
      (if (< b c) 
          (result a b c)
          (if (< a c) (result a c b) (result c a b)))
      (if (< a c) 
          (result b a c)
          (if (< b c) (result b c a) (result c b a)))))

Is there a particular reason that you didn't define the final statement as a function that accepts three arguments (or a three-element list), and then call it? Also, is there any reason that your function prints out a summarizing sentence of its results, rather than returning a sorted list? Both of these points severely limit the reusability of your code. Had you, for example, defined this as my-sort-3, instead of making it a naked let call, you could have used it in your triangle-related assignment.

This would also further simplify the code

(defun my-sort-3 (a b c)
  (if (< a b)
      (if (< b c) 
          (list a b c)
          (if (< a c) (list a c b) (list c a b)))
      (if (< a c) 
          (list b a c)
          (if (< b c) (list b c a) (list c b a)))))

(format t "The sorted result is ~a~%" 
          (apply #'my-sort-3 (loop repeat 3 collect (random 25))))
\$\endgroup\$
  • \$\begingroup\$ Thank you! This looks fantastic. I see how redefining it that way makes it more elegant and reusable. I am a bit confused about how it would be used in the triangle code, but otherwise I am very happy with this feedback - thank you. \$\endgroup\$ – jaresty Mar 7 '11 at 23:52
  • \$\begingroup\$ @jaresty - I left a pretty extensive answer there too (and it explains where you would use this) :) \$\endgroup\$ – Inaimathi Mar 7 '11 at 23:58
  • \$\begingroup\$ Sorry - I just got to looking at the triangle explanation and I clearly see it written there. Thanks! \$\endgroup\$ – jaresty Mar 8 '11 at 5:36
  • \$\begingroup\$ By the way, why do you need the 'apply' in the last example? Would it work the same if written "(my-sort-3 (loop repeat 3 collect (random 25)))"? \$\endgroup\$ – jaresty Mar 9 '11 at 3:01
  • \$\begingroup\$ It will if you have my-sort-3 take a list as its argument. If you want it to take three numeric arguments, you have to apply it (or get into multiple-value returns and calls). \$\endgroup\$ – Inaimathi Mar 9 '11 at 3:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.