Return Styles: Pseud0ch, Terminal, Valhalla, NES, Geocities, Blue Moon. Entire thread

LISP Code Style

Name: Anonymous 2009-11-03 1:50

Hi.  I'm sortof new to LISP, so I'm not that good at it, but i wrote a program that converts decimal numbers into Roman numerals.  It's not that exciting of a programming, but I like making unique little programs that I can use at the oddest times.  LISP is an awesome language, but I think it's harder to read (i made a game in C before this so i'm a C programmer).  I decided to use the C style to make the code look more organized.  I think it makes it easier to read.  It's not GNU style, but I was wondering if the GNU has a format guideline for LISP.  I was thinking of submitting something like this, because it makes LISP code easier to read.

(
    defun roman1
    (
    )
      "Roman numeral conversion with an unordered P.S."
      (
        let
        (
            (
            x nil
            )
        )
            (
            loop
                  (
                cond
                        (
                    (
                        null x
                    )
                    (
                        format t "Enter number:"
                    )
                    (
                        setf x
                        (
                            read
                        )
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                        (
                            > x 39
                        )
                    )
                             (
                        format t "too big~%"
                    )
                    (
                        setf x nil
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                        (
                            < x 40
                        )
                        (
                            > x 9
                        )
                    )
                             (
                        prin1 'x
                    )
                    (
                    setf x
                        (
                            - x 10
                        )
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                        (
                            = x 9
                        )
                    )
                             (
                        prin1 'ix
                    )
                    (
                        setf x 0
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                        (
                            < x 9
                        )
                        (
                            > x 4
                        )
                    )
                             (
                        prin1 'v
                    )
                    (
                        setf x
                        (
                            - x 5
                        )
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                        (
                            = x 4
                        )
                    )
                             (
                        prin1 'iv
                    )
                    (
                        setf x 0
                    )
                )
                        (
                    (
                        and
                        (
                            not
                            (
                                null x
                            )
                        )
                         (
                            < x 4
                        )
                        (
                            > x 0
                        )
                    )
                             (
                        prin1 'i
                    )
                    (
                        setf x
                        (
                            1- x
                        )
                    )
                )
                        (
                    (
                        zerop x
                    )
                    (
                        setf x nil
                    )
                    (
                        terpri
                    )
                )
                     )
        )
    )
)

Name: Anonymous 2009-11-03 11:27

In the unlikely even that you are not trolling:

Your indentation/formatting style is non-idiomatic. Please read http://dept-info.labri.fr/~strandh/Teaching/MTP/Common/Strandh-Tutorial/indentation.html for indentation, and http://mumble.net/~campbell/scheme/style.txt (Scheme specific, but still important). Get a good editor like Emacs, SLIME for interaction with your Lisp and Paredit for structured editing. You do get plenty of freedom on how you split lines, but the actual formatting is shared between most Lisp programmers, because people don't read code by counting parens, but they read it by indentation and general positioning of your code.


Here's your code reformatted using Emacs:

(defun roman1 ()
  "Roman numeral conversion with an unordered P.S."
  (let ((x nil))
    (loop
       (cond
         ((null x)
          (format t "Enter number:")
          (setf x (read)))
         ((and (not (null x))
               (> x 39))
          (format t "too big~%")
          (setf x nil))
         ((and (not (null x))
               (< x 40)
               (> x 9))
          (prin1 'x)
          (setf x (- x 10)))
         ((and
           (not
            (null x))
           (= x 9))
          (prin1 'ix)
          (setf x 0))
         ((and (not (null x))
               (< x 9)
               (> x 4))
          (prin1 'v)
          (setf x (- x 5)))
         ((and (not (null x))
               (= x 4))
          (prin1 'iv)
          (setf x 0))
         ((and (not (null x))
               (< x 4)
               (> x 0))
          (prin1 'i)
          (setf x (1- x)))
         ((zerop x)
          (setf x nil)
          (terpri))))))

Your code is not very idiomatic as well, and shows that you probably did not study Lisp from a good book (or didn't finish reading the book).
Here's some pointers:
Practical Common Lisp (freely available) or ANSI CL, and look
up the functions in the Hyperspec.
http://norvig.com/luv-slides.ps for general Lisp coding style questions.


Here's my attempt at solving your problem:

;;;; Some common utils

(defun flip (x)
  (destructuring-bind (a b) x (list b a)))

(defun flatten (tree)
  "Traverses the tree in order, collecting non-null leaves into a list."
  (let (list)
    (labels ((traverse (subtree)
               (when subtree
                 (if (consp subtree)
                     (progn
                       (traverse (car subtree))
                       (traverse (cdr subtree)))
                     (push subtree list)))))
      (traverse tree))
    (nreverse list)))

;;;; Number conversion

(defparameter *roman-numerals-table*
  '((#\M 1000)
    (#\D  500)
    (#\C  100)
    (#\L   50)
    (#\X   10)
    (#\V    5)
    ((#\I #\V) 4)
    (#\I    1)))

(defun single-integer->roman (x) 
  (assoc x '#.(mapcar #'flip *roman-numerals-table*)
         :test #'(lambda (x element)
                   (>= x element))))

(defun integer->roman-numeral-list (x)
  (flatten
   (loop
      for i = (single-integer->roman x)
      while i
        for (value roman-numeral) = i
          collect roman-numeral
          do (decf x value))))

(defun integer->roman-numeral-string (x)
  (check-type x integer)
  (concatenate 'string (integer->roman-numeral-list x)))

;;; I don't write main functions too often, instead just use the REPL, but since you wrote one, here it is:
(defun main () 
  (loop
     (format t "Enter number:")    
     ;; Don't let the user execute code through user input
     (let (*read-eval*)
       (let ((x (read)))
         (if x            
             (format t "~&~A~&" (integer->roman-numeral-string x))
             ;; User entered 0, we're done
             (return nil))))))

I believe my code is more idiomatic Lisp, and it's more extensible than yours. You could for example, easily add support for multiples of 1000 in it, just by modifying *roman-numerals-table* to contain what is needed. Note that if you do so, you must redefine or recompile single-integer->roman as it caches the values.

Newer Posts
Don't change these.
Name: Email:
Entire Thread Thread List