# Tag Info

16

What is not enough LISP-y in your function is the style used for the parentheses (they should not be left alone as last element of a line and they should be on the last line of the enclosed form). Another minor point is that you can use the shorter mapcar instead of map 'list. Here is a version of your function (generalized for any function applied to the ...

10

You are not using tail recursion in my-sum, so the compiler cannot easily turn it into iteration. It depends on the specific problem; in this case I would probably do it iteratively. Sure, loop is useful and convenient. Here is my attempt: (defun multiple-of-3-or-5 (x) (or (zerop (mod x 3)) (zerop (mod x 5)))) (defun my-sum (from to) (loop :...

10

Notation It is a common convention in Lisp to avoid uppercase (or mixed lowercase and uppercase) letters in identifiers (symbols), so for instance use l instead of L, or fibo-nums instead of fiboNums, etc. Variables If you type your function in a Common Lisp interpreter/compiler, you will receive warnings undefined variable for A B and next. This is ...

8

Use primitive functions and operators as much as possible You are defining remove-dups, and in this function you use the primitive adjoin, which adds an element to a list if not already present. But in Common Lisp the primitive function remove-duplicates is also available, that returns a list without duplicates. Instead of (+ expression 1) use the ...

7

I agree with sds's post. In addition, you should note that eq does not guarantee to return true for two equal numbers (see, especially, the (let ((x 5)) (eq x x)) case). You should use zerop to test if a number is zero (and = for comparing numeric equality in general).

7

Do not recompute is-multiple repeatedly by either binding the value: (defun num-action (i) (let ((i3 (is-multiple i 3)) (i5 (is-multiple i 5))) (cond ((and i3 i5) (print "FizzBuzz")) (i3 (print "Fizz")) (i5 (print "Buzz")) (T (print i))))) or by using if: (defun num-action (i) (if (is-multiple i 3) (if (...

7

The IDE you linked uses CLISP, which is a bit lenient; when I evaluate that definition, I immediately get two warnings from SBCL: ; in: DEFUN COUNT-UP-DOWN-CHARACTERS-WITH-DIFFERENCE ; (SETF COUNT-DOWN 0) ; ==> ; (SETQ COUNT-DOWN 0) ; ; caught WARNING: ; undefined variable: COUNT-DOWN ; (SETF COUNT-UP 0) ; ==> ; (SETQ COUNT-UP 0) ; ; ...

7

This is the typical situation where a hash table that maps elements to their frequency count is particularly well suited. (defun occurrences (lst) (let ((table (make-hash-table))) ;  (loop for e in lst do (incf (gethash e table 0))) ;  (sort (loop for k being the hash-key of table ;  ...

6

Trivial Use 1- instead of (- ... 1). Avoid very long lines (Emacs will indent for you). Do not use cond when a single if without progn would do. Memory Use nconc instead of append when possible to avoid unnecessary consing (in your case, splice allocates a fresh list, so its result can be passed to nconc). Catastrophic Whenever you use nth, you are ...

6

Don't take all remarks as absolute. Some are just how I would do things. There are many ways to structure a program. It's a great way to learn Lisp programming! Good: documented constant values with speaking names names are usually speaking and self-documenting use of CLOS use of documentation strings Suggestions for improvements: DEFCLASS has a :...

6

There are several problems with your code. Style is-prime is C/Java style. Lispers use primep or prime-number-p. zerop is clearer than (= 0 ...). Lispers use indentation, not paren counting, to read code. Your code is thus virtually unreadable. Please use Emacs if you are unsure how to format lisp properly. Stack overflow is-prime is tail-recursive, so ...

6

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

6

The aspect of the code that may cause difficulty to those new to Lisp appears to be the fact that in Common Lisp, nil has two meanings. It's the value of an empty list () and it's also false when used in boolean context. So when we have something like (or '(1 2 3) ()) the first non-NIL list is returned. When we use and it returns the last item if all ...

6

You are asking: which version would you favour and why? Instead of answering directly to your question, I will try to show why that formulation is not, at least in my opinion, “a relic of former days”, but quite idiomatic for Common Lisp (and other “terse” languages). My attempt is done by first recalling an important concept of the language, and then by ...

5

Lisp is a multiparadigm language. apply is just as lispy as recursion, and, in a way, much more so (think in HOFs)! Style Please fix indentation. Please write #'foo instead of (function foo). Implementations The first (HOF) version can be much more efficiently rewritten in using mapcan (provided defscribe-path returns fresh lists): (defun describe-...

5

Superficial Your find-node is actually (almost) assoc or, if you prefer, (find node graph :key #'first). Use mapcar instead of map 'list because it makes the intent clearer (the difference is that mapcar takes only lists and map any sequence). You need just one do in loop; you can even fold all your setfs into one (setf a b c d e f). In loop, with should ...

5

First of all, I suggest that you use a structure instead of a hash table when you know that the table will only have 3 elements; this improves both readability and performance: (defstruct triplestore-graph (spo (make-hash-table)) (pos (make-hash-table)) (osp (make-hash-table))) (defvar *triplestore-graph* (make-triplestore-graph)) Next, there is no ...

5

(let ((numbers (range 3 (+ limit 1) 2)) Above defines a list of numbers. Later you set various numbers to zero. That's not good for a list structure. Use a vector, an one-dimensional array. (dolist (step (range 3 (+ limit 1) 2)) Creating the list and then iterating over it makes no sense. Use a LOOP statement directly. (dolist (i (range start half step)...

5

Overall The code achieves the most important goal. It runs and produces the correct answer. Using helper/auxiliary methods is a very useful practice and makes the code clearer. Naming As mentioned in the accepted answer, it is standard practice to use lower case for all names. The reason is that historically, the Lisp pretty printer prints in ALLCAPS. ...

5

A possible way is to use one of the bitwise logical operators on integers, that treat integers as binary numbers. For instance, by using the logxor operator, we could write: (defun invert-bits2 (n) (if (> n 0) (logxor (1- (expt 2 (integer-length n))) n) 0)) The function integer-length returns the number of bits of the binary representation ...

5

For loop you can see the excellent book by P. Seibel “Practical Common Lisp”, available on-line, in particular chapter 7 and chapter 22. Let’s start from the last function: the idea is ok, we can just simplify the function noting that (lambda (x y) (+ x y)) is nothing more than the original +: (defun pos+3 (lst) (mapcar #'+ lst (range (length lst)))) ...

5

There are some cases to be considered. Though we can write it slightly different: CL-USER 32 > (let ((a #(1 5 8 10 11)) (b #(1 2 6 7 10))) (flet ((merge- (x y &aux (lx (length x)) (ly (length y)) (lc (+ lx ly)) (c (make-array lc)) ...

4

In list->num you can count down with something like for i downfrom n. (defun num->list (n) (loop for c across (write-to-string n) collect (parse-integer (string c)))) In above function you can just collect (digit-char-p c). The function returns the digit value as a number.

4

DEFUN The most basic mistake in your code is that DEFUN is not correct for nested functions. DEFUN is a top-level macro, defining a top-level function and should be used in top-level forms. Nested functions are defined in Common Lisp with FLET and LABELS. LABELS is used for recursive sub-functions. Naming Symbols like FooBarBaz are not use in Common Lisp....

4

Use nconc instead of append for speed in (setf merged-list (append merged-list (list a))) Note that you need setf with nconc only if merged-list is nil. Use pop: instead of (let ((a (first lower))) (setf merged-list (append merged-list (list a))) (setf lower (rest lower)))) you can write (setf merged-list (nconc merged-list (list (pop lower)))) The ...

4

It looks okay for a recursion exercise, but it's naive code. Not usable in 'production'. it conses like mad. It creates a lot of intermediate garbage. Splitting a string involves making lots of smaller strings. for lists it is not efficient, too CONCATENATE in loops or recursive functions is a code smell the recursive implementation with the subroutine is ...

4

You're solving the problem inefficiently; in the same way that you can mentally figure out in a few seconds that the sum of every number from 1 to 100 inclusive is 5050, you can do the same here: 999/5 = 199 instances, divided by 2 is 99 (plus 500, the half-way mark) 5 + 995 = 1000 sum of opposite numbers 1000 sum * 99 instances = 99,000 ...

4

Is my use of recursion wrong? It's not wrong, but it's unnecessarily complex. There's no need to create a list and recurse over it when you could simply iterate over the numbers. Should I prefer iterative approach over recursion in Lisp too? (as in imperative languages) Iteration is perfectly acceptable in functional languages! Functional programmers ...

4

There are lots of minor improvements possible to your code. The biggest problems though is: unclear function interfaces. (defun output-consequent-layer (anfis prev-output input) I have no idea what anfis, prev-output or input actually is. Either write comments for those, document the basic data structures somewhere or actually do it in Lisp code. Type ...

4

(defun skewed-average1 (list) "calculate average by summing and dividing" (let ((sum 0) (n 0)) (dolist (x list) (if (>= x 0) (progn (if (> x 100) (setf x 100)) (setf sum (+ x sum)) (setf n (+ 1 n))))) (/ sum n))) IF ... PROGN is WHEN. >= 0 is plusp. (setf sum (+ ... is INCF. (defun ...

Only top voted, non community-wiki answers of a minimum length are eligible