CODEWITHSUNDEEP
haskell
Luke Morgan
Luke Morgan

Reputation: 1930

Count consecutive sublists with same length

I am trying to count the elements of a list that are the same length for n and n+1.

I wrote this code:

countSec :: [[Integer]] -> Integer
countSec (x:xs) = if (length x)==(length (head xs)) 
    then 1+(countSec xs) 
    else (countSec xs)
countSec [] = 0

As you may guess, it's not working. In fact, as a output I get "* Exception: Prelude.head: empty list"

countSec [[1,2][1,2],[2],[3,5],[2],[5]] should return 2 (the first two and the last two sublists have the same length).

Any clue on what the problem can be?

Thanks

Upvotes: 1

Views: 878

Answers (4)

solrize
solrize

Reputation: 11

There are some builtins you can use:

import Data.Function (on)
import Data.List (groupBy)

countSec = length . head . groupBy ((==) `on` length)

Upvotes: 1

ehird
ehird

Reputation: 40797

You don't handle the case where there is only one element in the list of lists, so you run into an error as soon as you call head on xs, which will be [], since there's no first element of an empty list :)

You can use pattern matching to fix this:

countSec :: [[Integer]] -> Integer
countSec [] = 0
countSec [_] = 0
countSec (x : xs@(y:_))
  | length x == length y = 1 + countSec xs
  | otherwise            = countSec xs

I used pattern matching to match two elements of the list at a time. This is just like x:(y:_) (patterns can nest), except that we also give the name xs to the thing we matched to y:_; so we have

             xs
x      y _____\_____
 \      \           \
  [1] : ([2, 3] : ...)

x is the first element of the list, y is the second, and xs is the list after the first element. We could just match (x:y:xs) instead and use countSec (y:xs) to recurse, but this would use up more memory and is more error-prone.

The _ in a pattern means that we don't care about the value at that position, and so don't want to give it a name; it's a wildcard that matches everything.

As a minor stylistic note, I also converted your if expression into a guard; these are basically just if expressions at the level of a function clause.

You should avoid using partial1 functions like head when pattern matching would work because of things like this — they hide errors and make code harder to read. I also suggest giving the -Wall flag to GHC; if you wrote countSec as I did but forgot the zero or one-element case, GHC would warn you about it:

Warning: Pattern match(es) are non-exhaustive
         In an equation for `countSec': Patterns not matched: [_]

1 A function not defined for all its inputs; for instance, head and tail are not defined on [], and division is not defined when the divisor is 0.

Upvotes: 6

Daniel Fischer
Daniel Fischer

Reputation: 183958

A different approach that calculates the length of each list only once,

countSec :: [[a]] -> Integer
countSec xss = genericLength . filter id $ zipWith (==) ls (tail ls)
  where
    ls = map length xss

The use of the partial function tail is perfectly safe here, because the case where it would raise an error is caught by the shortcutting of zipWith.

Upvotes: 3

aleator
aleator

Reputation: 4486

The tail part of your list, the xs will be [] when you reach the end of the list, and head will result in an error.

However, the general answer to this kind of question is "never use head. Use pattern matching". This will instantly show what is wrong in your program:

countSec :: [[Integer]] -> Integer
countSec (x:y:xs) = if (length x)==(length y) 
    then 1+(countSec (y:xs)) 
    else (countSec (y:xs))
countSec (x:[]) = ??  
countSec [] = 0

Upvotes: 2

Related Questions