Minor algorithmic improvements.#5
Conversation
MatthewRock
left a comment
MatthewRock
left a comment
There was a problem hiding this comment.
Thank you for your work! I've found some time to review the changes. My code leave a lot of room for improvement, so thanks for bearing with me. The changes look good; the only thing I would ask for is to add some comments; the code requires you to stop and think for a minute.
| (when k (string k))) | ||
| "")))) | ||
| (type list prefix)) | ||
| (let ((new-prefix |
There was a problem hiding this comment.
Please add some comments to make it easier to glance over this code.
| (recursive-fun trie "")) | ||
| (let* ((len (reduce #'+ new-prefix :key #'length)) | ||
| (string (make-string len))) | ||
| (loop for p in new-prefix |
There was a problem hiding this comment.
Alternatively we can move this let and loop into a FLET function, e.g. "prefix-to-key" (though I don't really like this name... throwing out the idea though)
| (list new-node (sort (cons new-node children-list) | ||
| #'char-greaterp :key #'key))))) | ||
| (values new-node | ||
| (if (null children-list) |
There was a problem hiding this comment.
Please add a comment to explain this operation (or better yet, create a named local function for this purpose).
| ;; Conditions | ||
| empty-key-warning | ||
| wrong-key-type-error)) | ||
| #:empty-key-warning |
There was a problem hiding this comment.
I'm curious - what are the practical implications of this? What are we gaining by not interning the symbols?
There was a problem hiding this comment.
The way I see it is that avoiding interning symbols is just polite: interned symbols are never garbage-collected, they result in noise when you call apropos, and they can result in needless package conflicts.
2 participants
This avoids using concatenate to build up strings, and avoids using sort to insert new items into the list of children.