tonymorris/fp-course

ListZipper nth is doing more traversal than is necessary in some cases

Opened this issue · 2 comments

In the case where the nth element on the left hand side, the entire left list is visited again via length, and none of the moving work that was already done is reused.

I think the following solution is near-optimal, but it may be possible to do better by keeping around the partial lists while you're trying to find the head. Sorry it's not very pretty, I'm doing this course to try to get better at haskell.

nth ::
  Int
  -> ListZipper a
  -> MaybeListZipper a
nth n origlz | n < 0 = IsNotZ
             | otherwise =
                  let (topList, llen) = countToHead origlz 0 in
                  if llen > n then moveRightN n topList
                  else moveRightN (n - llen) origlz
  where countToHead lz lacc = case moveLeft lz of
                              IsZ nlz -> countToHead nlz (lacc + 1)
                              IsNotZ  -> (lz, lacc)

Thanks for putting the course together. It's been great fun so far!

Actually, you may be able to do even better by checking if it's halfway up the left list or more and then use the original listzipper to move backwards.

So, something like:

nth ::
  Int
  -> ListZipper a
  -> MaybeListZipper a
nth n origlz | n < 0 = IsNotZ
             | otherwise =
                  let (topList, llen) = countToHead origlz 0 in
                  if llen > n then
                     if llen `div` 2 < n then moveRightN n topList
                     else moveLeftN (llen - n) origlz
                  else moveRightN (n - llen) origlz
  where countToHead lz lacc = case moveLeft lz of
                              IsZ nlz -> countToHead nlz (lacc + 1)
                              IsNotZ  -> (lz, lacc)