FizzBuzz in Common Lisp

Question

Where I stand on Lisp

So, Lisp. It's been on my radar for years and I'm finally getting around to learning it. The syntax is foreign but I find I'm having the most difficulty in the areas of semantics and the standard library. The symbol semantics are giving me tremendous trouble for sure, mainly because I find it hard to tell when a symbol is needed as opposed to a value. It probably sounds silly, but I find C pointers to be more intuitive and straightforward, and this is probably owed to the fact that C is exclusively pass-by-value, whereas Lisp is ostensibly both pass-by-value and pass-by-reference, and the semantics therein--which behavior is chosen and when--is unknown to me. For example, why is (1 2 3) inappropriate in a situation where '(1 2 3) and list--assuming its value is (1 2 3)--are perfectly okay? Ostensibly, this is an ambiguity in the syntax and Lisp thinks I'm trying to call a function called 1. This type of situation stresses me out precisely because I start thinking of the unknown unknowns--what other situations like this are going to come along and derail me or cause bugs that remain hidden for extended periods of time? I imagine, perhaps without warrant, disasters on par with Undefined Behavior in C.

The code to be reviewed

Long intro, but since this is a code review, I feel it should probably be made clear where I stand so that reviews can take it into account and use it to more effectively communicate the review. I.e. I am a seasoned programmer, I dabble in "language lawyering", and I'm a complete beginner in Lisp.

That said, my first Common Lisp project is FizzBuzz. And what better way to get more comfortable with the language than to put my newbie code out for other people to systematically tear apart?

#|| divisible()
 || I don't usually write functions for something as obvious as
 || "is X divisible by Y", but in this case, I find
 ||
 ||     (divisible 15 5)
 ||
 || easier to read than
 ||
 ||     (= 0 (mod 15 5))
 ||
 || feel free to refute if you disagree and think this is a
 || code smell or you know of a more idiomatic way
 ||
 || Also feel free to say something if you feel
 || this C-like multi-line style is in bad taste
 ||#
(defun divisible (a b) (= 0 (mod a b)))

#|| range()
 || Copy/pasted from somewhere else. I don't know what cons does
 || I do understand that the function is recursive
 || I do not think I would have thought of this on my own
 ||#
(defun range (min max)
  (when (<= min max)
    (cons min (range (+ min 1) max))))

(defun fizzbuzz (n)
  (setf out (string ""))

  (unless (or (divisible n 3) (divisible n 5))
    (return-from fizzbuzz n))

  (when (divisible n 3)
    (setf out (format nil "~d~d" out "Fizz")))

  (when (divisible n 5)
    (setf out (format nil "~d~d" out "Buzz")))

  out)

(setf 1to100 (range 1 100))
(setf 1to100fb (map 'list #'fizzbuzz 1to100))
(mapcar #'(lambda (str) (format t "~d~%" str)) 1to100fb)

I admit, it's hard to break away from imperative programming.

The style isn't completely foreign to me, actually. I've already used (and am very much a fan of) style similar to this in C-like languages...

while(condition)
    if(condition)
        do_something;

And I'm already a fan of functional programming...

return list.map(x => (x * x) % 256).filter(x => x >= 10);

So I believe I'm on the right track. Although Dunning-Kruger is always in effect.

Answer 1

1. Divisible function

It is perfectly fine to define a function like divisible to improve the reading of a program. In general, when defining a function that could be reused, it is a good practice to comment its meaning (and this is encouraged by the syntax of the language):

(defun divisible (a b)
  "a is evenly divisible by b"
  (zerop (mod a b)))

Note the idiomatic use of the predicate (zerop x) instead of (= 0 x).

2. Introduction of variables

The use of setf (or setq) to introduce new variables is wrong (you should have received a warning, something like In FIZZBUZZ: Undeclared free variable OUT). The setf and setq operators should be used only to modify them.

Instead, you can introduce local variables with the let syntax:

(let ((new-local-variable1 initial-value)
      (new-local-variable2 initial-value)
      ...)
  body-in-which-the-new-local-variables-are-bound)

So fizzbuzz could be redefined as:

(defun fizzbuzz (n)
  "return n or Fizz or Buzz or FizzBuzz"
  (let ((out (string "")))

    (unless (or (divisible n 3) (divisible n 5))
      (return-from fizzbuzz n))

    (when (divisible n 3)
      (setf out (format nil "~d~d" out "Fizz")))

    (when (divisible n 5)
      (setf out (format nil "~d~d" out "Buzz")))

  out))

The global variables are instead introduced by defvar, so you could write for instance:

(defvar 1to100 (range 1 100))
(defvar 1to100fb (map 'list #'fizzbuzz 1to100))

3. The algorithm

The function range actually builds a list of (max-min+1) elements, filled with the values min, min+1, min+2, ..., max. The use of map over 1to100 is quite an expensive way of performing a simple iteration from the integer min to the integer max! Others, much more efficient ways, exist. For instance, you could do a classical loop (think of a for in a C-like language):

(loop for i from 1 to 100
  do (format t "~d~%" (fizzbuzz i)))

Another possibility is to use the dotimes operator:

(dotimes (i 100)
   (format t "~d~%" (fizzbuzz (1+ i))))

that iterates from 0 to (but not included) 100 (so there is the need of use the 1+ function).

If, on the other hand, you want to go for recursion, here is a possible solution (which is tail-recursive, so compiled by most systems as an iterative loop):

(defun fizzbuzz-to (limit &optional (start 1))
  (when (<= start limit)
    (format t "~d~%" (fizzbuzz start))
    (fizzbuzz-to limit (1+ start))))

4. Types

In a format string, for a general printing ~a should be used instead of ~d, which mainly is used to print integer (decimal) numbers. It can be used also for strings, but this is due to the fact that in the specification it is said that:

if arg is not an integer, it is printed in ~A format and decimal base.

What I do not like more, however, is the fact that the function fizzbuzz sometimes returns an integer, sometimes a string. The fact that Common Lisp has dynamic typing in some way hides this fact, but I think that the program would be more elegant (and in some sense more correct) if the result has always the same type.

One way is to redefine the function in this way:

(defun fizzbuzz (n)
  (cond ((divisible n 3)
         (if (divisible n 5)
             "FizzBuzz"
             "Fizz"))
        ((divisible n 5) "Buzz")
        (t (format nil "~d" n))))

For other solutions, you could for instance look at this CR question.