patternMinor
Mergesort with an inversion counter
Viewed 0 times
withinversioncountermergesort
Problem
I'm new to Lisp and I'm yet to wrap my head around the lisp way of writing programs. Any comments regarding approach, style, missed opportunities appreciated:
In particular, please advice if I build results list correctly (
In particular, please advice if I build results list correctly (
(setf merged-list (append merged-list (list a)))).;;;; Count inversions
(defun small-list (list)
(or (null list) (null (rest list))))
(defun split-in-half (list)
(let ((mid (ceiling (length list) 2)))
(values (subseq list 0 mid)
(subseq list mid))))
(defun count-inversions (list)
(if (small-list list) (list list 0)
(multiple-value-bind (lower upper) (split-in-half list)
(merge-inversions
(count-inversions lower)
(count-inversions upper)))))
(defun merge-inversions (lower-pair upper-pair)
(let ((lower (first lower-pair))
(upper (first upper-pair))
(merged-list '())
(num-inversions 0))
(loop while (not (and (null lower) (null upper)))
do (cond
((null lower) (let ((a (first upper)))
(setf merged-list (append merged-list (list a)))
(setf upper (rest upper))))
((null upper) (let ((a (first lower)))
(setf merged-list (append merged-list (list a)))
(setf lower (rest lower))))
((< (first lower)
(first upper)) (let ((a (first lower)))
(setf merged-list (append merged-list (list a)))
(setf lower (rest lower))) )
(t (let ((a (first upper)))
(setf merged-list (append merged-list (list a)))
(setf upper (rest upper))
(incf num-inversions (length lower))))))
(list merged-list (+ (second lower-pair) (second upper-pair) num-inversions)))Solution
-
Use
(setf merged-list (append merged-list (list a)))
Note that you need
-
Use
(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 combination of
-
Usually predicates are named with a -p suffix, so I suggest that you rename your
-
Please fix line breaks and indentation in the third
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 combination of
let and loop creates extra indentation for no good reason. You can use with clause in loop to create bindings instead, or use the do macro.-
Usually predicates are named with a -p suffix, so I suggest that you rename your
small-list to small-list-p.-
Please fix line breaks and indentation in the third
cond clause.Context
StackExchange Code Review Q#21431, answer score: 4
Revisions (0)
No revisions yet.