CODEWITHSUNDEEP
performancehaskellrecursionclojure
Orphid
Orphid

Reputation: 2852

Performance of iterating a list by recursively examining the tail

I decided to try to learn Haskell by doing some of the CodinGame challenges (so this question is super beginner-level stuff I'm sure). One of them requires searching through a list of integers for the smallest difference between any two values. I've previously solved it in Clojure by doing this:

(ns Solution
  (:gen-class))

(defn smallest-difference [values]
  (let [v (sort values)]
    (loop [[h & t] v curr-min 999999]
      (if (nil? t) curr-min
        (let [dif (- (first t) h)]
          (recur t (if (> curr-min dif) dif curr-min)))))))

(defn -main [& args]
  (let [horse-strengths (repeatedly (read) #(read))]
      (let [answer (smallest-difference horse-strengths)]
        (println answer))))

I tried to implement the same solution in Haskell, with the following:

readHorses :: Int -> [Int] -> IO [Int]
readHorses n h
    | n < 1 = return h
    | otherwise = do
        l <- getLine
        let hn = read l :: Int
        readHorses (n - 1) (hn:h)

findMinDiff :: [Int] -> Int -> Int
findMinDiff h m
    | (length h) < 2    = m
    | (h!!1 - h!!0) < m = findMinDiff (tail h) (h!!1 - h!!0)
    | otherwise         = findMinDiff (tail h) m



main :: IO ()
main = do
    hSetBuffering stdout NoBuffering -- DO NOT REMOVE
    input_line <- getLine
    let n = read input_line :: Int
    hPrint stderr n
    horses <- readHorses n []
    hPrint stderr "Read all horses"
    print (findMinDiff (sort horses) 999999999)
    return ()

This times-out for large inputs (99999 values) where the Clojure solution does not. They look pretty similar to me however.

It at least superficially seems that reading the values and constructing the list isn't the problem, as "Read all horses" is printed before the timeout.

How can I make the Haskell version more performant?

Upvotes: 2

Views: 135

Answers (2)

chi
chi

Reputation: 116139

By the way, a completely alternative implementation could be written as follows. (Pseudocode follows)

Take the list

h = [h0, h1, h2 ...

Remove one element

drop 1 h = [h1, h2, h3 ...

Compute pointwise differences

zipWith (-) (drop 1 h) h = [h1-h0, h2-h1, h3-h2, ...

Then take the minimum. Full code:

minDiff :: [Int] -> Int
minDiff h = minimum (zipWith (-) (drop 1 h) h)

Note that this will crash on the empty list. On the other hand, there's no need of the 9999999 hack. Thanks to laziness, it runs in constant space, too.

For a better error handling:

minDiff :: [Int] -> Int
minDiff [] = error "minDiff: empty list"
minDiff h = minimum (zipWith (-) (drop 1 h) h)

or even, more pedantically (but respecting totality):

minDiff :: [Int] -> Maybe Int
minDiff [] = Nothing
minDiff h  = Just (minimum (zipWith (-) (drop 1 h) h))

Upvotes: 4

Cirdec
Cirdec

Reputation: 24156

You're calculating the length of the list in every recursion into findMinDiff. Since length takes O(n) time findMinDiff takes O(n^2) time instead of O(n).

You can write the same thing with pattern matching instead of length, !!, and tail

findMinDiff :: [Int] -> Int -> Int
findMinDiff (h0 : hs@(h1 : _)) m = 
    if h1 - h0 < m
    then findMinDiff hs (h1 - h0)
    else findMinDiff hs m
findMinDiff _ m = m

Upvotes: 5

Related Questions