We’re rewarding the question askers & reputations are being recalculated! Read more.

Hot answers tagged

16

This is two and a half years old, but for the sake of future viewers: Overall your coding style looks pretty good, especially for someone new to Clojure. I don't have any comments on the AI engine itself, but here are a few structural notes: not-nil? already exists in clojure.core as some?. Docstrings are usually phrased actively: e.g. "Returns the result ...


16

Here are my comments after a brief reading: First, if you intend to make this library fit for general use, add documentation in comments for the provided functions. See the Racket style guide for examples Also, if you intend to make this a more general library, don't include default arguments (e.g. for ircbot-connect) unless they make sense for all users. ...


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


9

I'm the author of core.matrix, so hopefully I can give you some tips from that perspective. If you want to improve performance, it's much better to use vectors in an optimised format throughout (vectorz-clj is a fine choice) rather than mixing in Clojure vectors everywhere. This saves the overhead of converting to/from Clojure vectors all the time, which is ...


8

When I went through the WYAS48 I made the same decision of breaking the bits of eval off into smaller functions. I found this made testing easier since I could focus on just one function at a time. I find that having a function be one massive bunch of logic is not helpful. In an imperative language the equivalent would be a single function with a ton of if/...


8

Yes, it can be shortened by using defstruct. An example from the docs: (cl-defstruct person name age sex) (setq dave (make-person :name "Dave" :sex 'male)) ⇒ [cl-struct-person "Dave" nil male] (setq other (copy-person dave)) ⇒ [cl-struct-person "Dave" nil male] (eq dave other) ⇒ nil (eq (person-...


8

Expectations Lisp is well-known for it's REPL, so I'd expect to see at least the following 3 functions in any Lisp interpreter: read, evaluate and print. read takes a string and returns a form (a number, string, symbol, list, and so on). For example: read("(+ 4 5)") should return a list that contains the symbol + and the numbers 4 and 5. evaluate takes a ...


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


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


5

I would suggest handling a nil input explicitly. Then you can: strip out the and (in s's binding), and just recurse directly strip out the cons at the bottom, leaving only the append Here's the code I tested with (in Scheme, since I have neither Emacs nor a CL implementation installed): ;; Requires SRFI 26 (define (combos lst) (if (null? lst) '(()) ...


5

I will point out stuff that I'd do differently, though I am by no means a clojure expert, I do have some experience in it. Numbered items will reappear inside the code as comments. no DEF inside defn body def will define the var for your entire namespace as such, you will pollude your namespace with temporaries use LET instead no DO necessary inside the ...


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

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

return isalpha(c) || c == '\'' || c == '?' || c == '+' || c == '-' || c == '*' || c == '/' || c == '^' || c == '=' || c == '>' || c == '<' || c == '~'; This could be rewritten much more readably as return isalpha(c) || (c != '\0' && strchr("'?+-*/^=><~", c) != NULL); A good compiler will produce the same code for both. (Neither GCC nor ...


5

A function's docstring must go before its argument vector. You've written your docstrings after your argument vectors, which prevents your :preconditions from being recognized and doesn't work when you try to retrieve the docstring with tools like doc: (doc palindrome?) ;; ------------------------- ;; user/palindrome? ;; ([arg]) ;; nil Rather than ...


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


4

So either a is the minimum of the list in which case b and c or it's not and it's one of the numbers you want to keep. The other is the max of b and c. (define (f a b c) (if (or (> a b) (> a c)) (sum-of squares a (max b c)) (sum-of-squares b c))) (define (square x) (* x x)) (define (sum-of-squares a b) (+ (square a) (square b))) Of ...


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

Two important problems: Your function could be better named: it returns a list of the non-empty subsets of its argument, so it should be called something like non-empty-subsets. There's no docstring. I think the most natural way to implement this function is to implement subsets (which is straightforward) and then drop the empty subset: (defun subsets (...


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

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


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