current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 9 down vote favorite

The following function finds all local maxima.

A local maxima is an element of a list such that it's greater than either of its neighbor elements. An element with 1 neighbor is not a local maximum.

localMaxima :: [Integer] -> [Integer]
localMaxima (x:y:z:zs) = if (y > x && y > z) then y : localMaxima (y:z:zs) 
                         else localMaxima (y:z:zs)
localMaxima _          = []

Please critique it.

Examples:

ghci> localMaxima [1,3,2,55,2] 
[3,55]
ghci> localMaxima [1,3,2,55,1000000000]
[3]
share|improve this question
    
Non-recursive oneliner: locmax xs = map (\(x, y, z) -> x) . filter (\(x, y, z) -> x > y && x > z) $ zip3 (drop 1 xs) xs (drop 2 xs). –  Cthulhu 1 hour ago

1 Answer 1

up vote 8 down vote

You have redundant parentheses here: (y > x && y > z). You can also make your type more generic: localMaxima :: Ord a => [a] -> [a].

I think it looks a bit cleaner using an as-pattern and guard clauses. This way you don't need zs and can just write _.

localMaxima (x:rest@(y:z:_))
  | y > x && y > z = y : localMaxima rest
  | otherwise      = localMaxima rest
localMaxima _ = []

In fact, if you know that y > z then you don't need to check if z is a local maximum.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.