HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

A heart.lisp I wrote with my 6 year old daughter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
yearlispwithwrotedaughterheartold

Problem

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*)

Solution

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.")

Code Snippets

(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-line (line)
  (loop for (type num) on line by #'cddr
        do (loop repeat num 
                 do (write-char (ecase type
                                  (:s #\space)
                                  (:h #\❤️)))))
  (terpri))
(defun print-lines (data)
  ;; ensure there is a new line
  (format t "~&")

  (dolist (line data)
    (apply #'print-line line)))
(defun print-lines (data)
  (terpri)
  (dolist (line data)
    (print-line line)))
(defvar *data*
  '((:s 8 :h 8 :s 8 :h 8)
    ...
    (:s 19 :h 2)))

Context

StackExchange Code Review Q#150881, answer score: 5

Revisions (0)

No revisions yet.