- The tests use
assert, instead I'd recommend one of the existing frameworks, possibly also linking it into ASDF so thatasdf:test-systemworks. - Some of the source files correctly have
in-package, some don't - I'd suggest always specifying which package to use, even (or especially) if it's example code. example.lispdoesn't work for me on CCL (I'd also suggest testing code with at least two or more implementations if you want it to be widely used):#(6 5 3 2 4 1) is not an adjustable array.Adding:adjustable tto the definition of*arr*fixes that.binheap.lisphas some trailing whitespace and some tabs. Consider M-x delete-trailing-whitespace and M-x untabify.(floor (/ x y))could be simplified to(floor x y)here (note the second return value is gonna be different though).(not (= ind 0))will hopefully be optimised. If not, however,(not (eql ind 0))might be better (of course(/= ind 0)also exists, but still does numeric comparison).- I'd suggest adding a
keyto make the container more generic / match the standard sequence operators. This helps immensely when adding some objects and then using one of the attributes for comparison; it composes better than a singletestargument. - Lastly, CLOS is great and all, but strictly speaking all of the
methods could be functions and therefore be a bit quicker. Unless of
course you have plans to add more containers with the same interface.
Consider also annotating everything with types and looking at what the
compiler (well, SBCL only really) will tell you about problems when
compiling with higher optimisation settings
(
(declaim (optimize ...))). However, I'd say that unless you're very certain do not however put(declare (optimize ...))into the library code, it's easy to get that wrong. - Don't use
assertif the error isn't correctable by changing the value interactively. Like inmake-heap, both of those should be regular errors: Retrying won't fix the problem (that's a common restart established withassert) and changingvecortestisn't something you'd do interactively ... I think. So,check-typeanderrorwould be the way to go here. - For randomised testing there's AFAIK nothing like a standard package, look for Quickcheck clones or "random testing common lisp" probably.
- It's "Common Lisp", no dash :)
ind,*arr*,arr,arrval, etc. aren't great names. Especially in function signatures consider matching what the standard uses for similar purposes. I'm betting that it'sindexandarrayrespectively. Common Lisp has a tendency to have long names (for good!) and I'm (nitpicky) of the opinion that it's good style to match that (as much as possible).loopshould be replaced byiteratebecause it has more parentheses. Not kidding, it simply looks better.- The docstrings are suboptimal and again should match an existing
style.
I: * Heap instance...I haven't seen before and it doesn't even mention which parameter (name) it describes. Take some exemplary documentation as a guideline perhaps.