HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Mergesort with an inversion counter

Submitted by: @import:stackexchange-codereview··
0
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 ((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 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.