clus-data
clus-data copied to clipboard
phoe
Add nth implementation
CL shies away from recursive code, because tail recursion is never guaranteed by the standard to be optimized. Therefore, for long enough lists, this implementation may blow the stack on conforming implementations.
An example implementation of NTH would rather need to be implemented imperatively.
I've redone the implementation without using recursion, but I'm not sure how to link to the > operator. (I don't think bad loop for ranges are guaranteed to work, so I'd rather not drop the test.)
Following the overall style of the specification, there's no need to replace NTH with MY-NTH - everywhere example implementations are added, the functions are named with their original names.
One-branched IF may be replaced with WHEN.
The test with > may be replaced with (check-type num unsigned-byte) in the beginning of the function. UNSIGNED-BYTE is a type that maps to nonnegative integers.
More: for the sake of debugging, the original variable binding should not be modified. Instead of going (setf list ...) you should rather wrap the function body with (let ((new-list list)) ...) and do (setf new-list ...) inside. If you don't do this, any backtraces that will be printed will only show the modified value of LIST and not the original value that the function was called with.
Additionally, you can replace (setf new-list (cdr new-list)) with (pop new-list), discarding POP's return value.
This should work now. Sorry this is taking so long for a trivial function, this is my first public Lisp project.
No problem, I perfectly understand. I'm also trying to teach you a little bit of Lisp along the way. (:
With this post, I'll try to show you my general thought process when reviewing and writing Lisp code, and also reference the specification whenever possible.
Your code has too many closing parens. Are you using any kind of paren-counting tool for your Lisp writing, such as paredit or smart-parens in emacs, or any kind of matching paren highlighting in other editors? It should even work with the DokuWiki markup, since the additional brackets will be matched as well.
After cleaning the unnecessary closing parentheses, the code you have posted is:
(defun my-nth (num list)
(let ((new-list list))
(when (check-type num unsigned-byte)
(loop for x from 1 to num do (pop new-list)))
new-list))
This function will not do what it is intended to do. The CLHS entry for CHECK-TYPE states that this macro always returns NIL, therefore the LOOP will never be executed, because (when (check-type ...) ...) will effectively mean (when nil ...).
I think I wasn't exactly clear about how CHECK-TYPE was meant to be used, so forgive me that one. CHECK-TYPE is meant to be used as an assertion inside the body of code. Treat it as a means of saying, "the variable NUM is expected to hold a value of type UNSIGNED-BYTE, and if it is not, we are in trouble, therefore signal an error in the program".
According to the specification of NTH, we want to signal error if NUM is not an UNSIGNED-BYTE, therefore I would use CHECK-TYPE since it already behaves as we want to. CHECK-TYPE is not meant to be used as a "soft" type check inside the program though; for getting a simple boolean answer of whether a value is of any given type, use TYPEP.
Additionally, you return NEW-LIST at the end of your function. NTH is meant to return the nth element of the list, not the nth tail of the list, like NTHCDR does. Therefore you should rather return the CAR of NEW-LIST.
Taking the above into account, I have changed your code into the following:
(defun nth (num list)
(check-type num unsigned-byte)
(let ((new-list list))
(loop for x from 1 to num do (pop new-list))
(car new-list)))
For testing this function, I will want to evaluate this form. To avoid replacing the original CL:NTH function, I will want to change the name of the function into something like MY-NTH. Once (defun my-nth ...) is evaluated, I can proceed with doing some basic tests of this function:
CL-USER> (my-nth 3 '(0 1 2 3 4 5))
3
CL-USER> (my-nth 0 '(0 1 2 3 4 5))
0
CL-USER> (my-nth 10 '(0 1 2 3 4 5))
NIL
CL-USER> (my-nth "two" '(0 1 2 3 4 5))
; Error: The value of NUM is "two", which is not of type UNSIGNED-BYTE.
CL-USER> (my-nth 2 "0 1 2 3 4 5")
; Error: The value "0 1 2 3 4 5" is not of type LIST.
These few manual tests assure me that my function works as I want it to.
Yeah, I see the use of check-type now. Thanks. I use Emacs with coloured delimiters, though that last commit was probably too rushed.
No problem. Take your time, and, first and foremost, always test your code locally before you submit it.
(I've made this mistake too many times myself. :)
There, I've updated my code to fit your suggestions. Thanks a million for the advice :)
Important question. What you mean by "test your code localy"? I supposed that can mean anthing from playing with code do write unit and integrate test for it. Do I understand it correctly?
@phoe demonstrated by loading the function into a REPL and running test inputs on it.