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

Sum of all multiples of 3 or 5 below 1000 (Project Euler #1 - typical)

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

Problem

I'm learning LISP and am starting with Project Euler. I would love some initial feedback on my LISP code for this simple task.

I know it spits out the correct answer, but what I'm not sure about is if I'm using LISP the way it is intended (and the way to strive for in the future in order to achieve success with LISP).

(defun sum (L)
  "sum a list"
  (apply '+ L)
)

(defun range (max &key (min 0) (step 1))
  "range of numbers from min to max by jumps of step"
  (loop for n from min below max by step
    collect n)
)

; take a range that steps by 3's and a list that steps by 5's and take the union of them (union removes duplicates)
(setq answer (sum (union (range 1000 :step 3) (range 1000 :step 5))))
(print answer)

Solution

Minor

Paren placement

Hanging parens are an eyesore.

Global var

Use defvar instead of setq to create global variables.

Sharp-quote for functions

You should do (apply #'+ ...) instead of (apply '+ ...).

Unnecessary allocation

Since your range returns a fresh list, you can use nunion instead of union.

Major

Sum

Your sum function is broken - it will not work on long lists, see CALL-ARGUMENTS-LIMIT.

Here are better ways to do this:

(defun sum (list)
  (reduce #'+ list))

(defun sum (list)
  (loop for x in list sum x))


Performance

union is probably quadratic in your implementation, so you might want to consider another approach for larger values of 1000 :-)

Code Snippets

(defun sum (list)
  (reduce #'+ list))

(defun sum (list)
  (loop for x in list sum x))

Context

StackExchange Code Review Q#150175, answer score: 3

Revisions (0)

No revisions yet.