Rumen Hristov
Rumen Hristov

Reputation: 887

Why does this index-selecting code not work?

I am trying to only return the odd-indexed elements of a list.

I have this code, it compiles, but it give me a run-time error.

here's my code:

oddSelect :: [Integer] -> [Integer]
oddSelect []     = []
oddSelect (x:xs) = head (drop 1 (x:xs)) : oddSelect (xs)

Upvotes: 2

Views: 92

Answers (3)

Mark Karpov
Mark Karpov

Reputation: 7599

If you want to use recursion:

evenSelect :: [a] -> [a]
evenSelect []       = []
evenSelect [x]      = [x]
evenSelect (x:_:xs) = x : evenSelect xs

Another solution which I like more:

import Data.List.Split

evenSelect :: [a] -> [a]
evenSelect = map head . chunksOf 2

Upvotes: 3

Arnon
Arnon

Reputation: 2237

Personally, I don't think that that's a good solution. Mostly, I am usually wary of using head.

What I'd do personally is zip the elements with an index, then remove all even numbers (filter odd) and then finally get rid of the indexes (map snd)

oddSelect :: [Integer] -> [Integer]
oddSelect l = map snd $ filter (\(idx,value) -> odd idx) (zip [1..] l)

As for your code, This clause is fine

oddSelect []        =[]

But the second one needs some work. I've rewritten it for you:

oddSelect (x:xs)    =x:oddSelect (drop 1 xs)

x is already the first element of the list (not a list by itself) so it can be added to the head of your new list. Now, you want to call your function again recursively, but now you want to snip the first element of the REST of the list, because it is even - so you want to call drop 1 xs instead of drop 1 (x:xs) which is equivalent of just using xs

Upvotes: 1

bereal
bereal

Reputation: 34312

drop 1 (x:xs) is actually the same as xs. If xs is empty, head must fail. Actually, even if you fix that part, it still won't work, since at this point you're trying to unconditionally drop the first element and continue recursively, which will always return an empty list.

The simple way is:

oddSelect = map snd . filter (odd . fst) . zip [1..]

But if you want your own recursive function, you still need to pass the current index somewhere or have some other way to drop only every second element.

Upvotes: 0

Related Questions