CODEWITHSUNDEEP
lispcommon-lisp
sarah
sarah

Reputation: 11

Creating a lisp file that reads numbers from the keyboard until 0 is read. Display the minimim value

I am stuck, I did it this way but I feel it is wrong. 

(defun (find-min a b))
   (cond 
        (> a b) (find-min b (read))
        (< a b) (find-min a (read))
        )
   (display (find-min (read) (read)))

Upvotes: 1

Views: 254

Answers (1)

coredump
coredump

Reputation: 38809

Code review

The code as given is badly formatted. It is really important to test your code often, to check that at least the syntax is correct. Instead of writing the code in isolation, you should ask the environment, the interpreter, to parse your code and execute it. You'll have a shorter feedback loop, which will help getting the code right.

For example, if you start your interpreter (you mentionned clisp), you are in a Read Eval Print Loop (REPL), where you write Lisp forms, which are evaluated to produce a result, which is printed. So just by writing code, it will get parsed and evaluated, which can quickly inform you about possible errors.

So if you write the code as you wrote it, you will notice that the interpreter first reads this:

(defun (find-min a b))

This is because all parentheses are balanced: the Lisp reader (the R in REPL) will build a list of two elements, namely defun and the list (find-min a b). When evaluation is performed (the E in REPL), the list will be interpreted as a Lisp form, but this is a malformed defun.

DEFUN expects a function name, followed by a list of variable bindings, followed by the function body. So you can try writing for example:

(defun find-min (a b)
  0)

This above defines a function named find-min, which accepts two parameters a and b, and always evaluate as 0 (the body of the function).

In your case you want to write:

(defun find-min (a b)
  (cond ...))

You COND form is malformed too:

   (cond 
        (> a b) (find-min b (read))
        (< a b) (find-min a (read))
        )

Each clause in a cond should be a list containing first a test, then zero or more expressions. So it would be:

(cond
  ((> a b) ...)
  ((< a b) ...))

Note that there is a remaining case, namely (= a b), which is not addressed.

Approach

But overall, I do not understand what your code is trying to achieve. Since all inputs are supposed to be obtained with (read), there is no need to accept arguments a and b. The whole thing could be a loop:

(defun read-min ()
  (loop
    for val = (read)
    for zerop = (= val zero)
    for min = val then (if zerop min (min min val))
    until (= val 0)
    finally (return min)))

As suggested by @leetwinski, there is also a shorter alternative:

(loop for val = (read) until (= val 0) minimizing val)

But I suppose you are not expected to use the loop form, so you should try to write a recursive function that does the same.

What you need to do is to pass the current minimum value ever read to your function, recursively, like so:

(defun read-min-rec (min-so-far)
  ...)

Upvotes: 3

Related Questions