7
\$\begingroup\$

Is it possible to improve this program somehow? It will probably be more convenient to suggest edits as forks of this Gist.

(defpackage heart
  (:use :cl))
(in-package :heart)

;; Usage:
;; sbcl -l heart.lisp -e '(uiop:quit 0)'
;;

(defun print-line (&rest args)
  (loop
     :for type :in args :by #'cddr
     :for num :in (cdr args) :by #'cddr
     :do (loop :repeat num
            :do (princ (ecase type
                         (:s " ")
                         (:h "❤️")))))
  (format t "~%"))


(defun print-lines (data)
  ;; ensure there is a new line
  (format t "~&")

  (dolist (line data)
    (apply #'print-line line)))


(defvar *data*
  '((:s 8 :h 8 :s 8 :h 8)
    (:s 3 :h 15 :s 4 :h 15)
    (:s 1 :h 18 :s 2 :h 18)
    (:s 0 :h 40)
    (:s 0 :h 40)
    (:s 0 :h 40)
    (:s 1 :h 38)
    (:s 2 :h 36)
    (:s 4 :h 32)
    (:s 6 :h 28)
    (:s 8 :h 24)
    (:s 11 :h 18)
    (:s 13 :h 14)
    (:s 15 :h 10)
    (:s 17 :h 6)
    (:s 19 :h 2)))

(print-lines *data*)
\$\endgroup\$
4
\$\begingroup\$

That's a great start! Looks like a lot fun.

I would do only a few things slightly different. Some of them a real improvements and some are style preferences of an old Lisper... ;-)

First function: PRINT-LINE

(defun print-line (&rest args)
  (loop
     :for type :in args :by #'cddr
     :for num :in (cdr args) :by #'cddr
     :do (loop :repeat num
            :do (princ (ecase type
                         (:s " ")
                         (:h "❤️")))))
  (format t "~%"))

There are two real improvements:

  • don't use a &rest arg, pass the list directly
  • the outer LOOP can be written slightly simpler with destructuring

New version:

(defun print-line (line)
  (loop for (type num) on line by #'cddr
        do (loop repeat num 
                 do (write-char (ecase type
                                  (:s #\space)
                                  (:h #\❤️)))))
  (terpri))

The second function: PRINT-LINES

(defun print-lines (data)
  ;; ensure there is a new line
  (format t "~&")

  (dolist (line data)
    (apply #'print-line line)))

There is one real improvement:

  • don't use apply, just pass the list directly

New version:

(defun print-lines (data)
  (terpri)
  (dolist (line data)
    (print-line line)))

The data variable

(defvar *data*
  '((:s 8 :h 8 :s 8 :h 8)
    ...
    (:s 19 :h 2)))

It helps to add a documentation string, or to give it a useful name.

(defvar *heart*
  '((:s 8 :h 8 :s 8 :h 8)
    ...
    (:s 19 :h 2)))

or

(defvar *data*
  '((:s 8 :h 8 :s 8 :h 8)
    ...
    (:s 19 :h 2))
  "The description of a heart.")
\$\endgroup\$
  • \$\begingroup\$ Thank you, Rainer! I didn't know about destructuring in the loop macro. Your version is much cleaner! \$\endgroup\$ – Alexander Artemenko Dec 27 '16 at 12:24

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.