Using quickCheck for "99 Haskell problems" - problem 1

Question

I'm working on learning Haskell, and I've implemented a solution to the first of the 99 Haskell problems. Here is my code:

import Data.Int
import Test.QuickCheck
myLast :: [a] -> Either String a
myLast [] = Left "Empty lists have no last element."
myLast xs = Right ((reverse xs) !! 0)
testMyLast :: [Int] -> Bool
testMyLast xs =
 case myLast xs of
 Left err -> null xs
 Right x -> x == last xs
main = quickCheck testMyLast

Now, I realize that my implementation of myLast is not the most efficient one. I'm not particularly interested in that type of feedback. Rather, I'm more curious about whether or not I'm following reasonable Haskell code style practices (the naming of things, indentation, etc.), as well as how I'm dealing with errors.

It's my understanding that using Either (or Maybe) when an error occurs is, in general, to be preferred over error "...", since dealing with exceptions is really only cleanly done in an IO context. Is my understanding correct? If it is, why do basic library functions (like List's last, for example) use error instead?

Answer 1

Most important point: Your answer does not do what is asked.

Question:

Prelude> myLast [1,2,3,4]
4
Prelude> myLast ['x','y','z']
'z'

Your implementation:

*Main> myLast [1,2,3,4]
Right 4

You should quickCheck for the equivalence of the function you are asked to implement myLast xs == last xs (with appropriate type signatures), or write a unit test suite comprising at least the cases given.

Other points:

• import Data.Int is unused.
• (xs !! 0) should be head xs

• Nested parentheses should be used sparingly because trying to match them reduces readability. Composition using operators . and $ should be preferred:

  -- instead of this
  myLast xs = Right (head (reverse xs))
  -- this reads better
  myLast' xs = Right $ head $ reverse xs
  -- this makes the composition of 3 steps more explicit
  myLast'' = Right . head . reverse
  

• CmdrMoozy: The other points are all exceptionally helpful, thanks! As for not doing what the question asked, this was a conscious decision. I'm wondering if my way is considered better or worse style in general, and why?
• Carcigenicate: @CmdrMoozy If you look at similar functions like head, it will crash if you attempt to use it on an empty list. The problem with wrapping the result in something is afterwords, you'll need to extract it, which for something petty like this isn't worth it. Also, for something like this that only has one possible "bad scenario", Maybe might be a better wrapper choice because you already know what went wrong if it crashes.
• abuzittin gillifirca: @CmdrMoozy If they hadn't given the example cases I would go with Maybe as Carcigenicate suggested, restating returning Maybe indicates an operation that may or may not be successful, whereas Either indicates some more of an operation that can encounter an irrecoverable error. You may ask your question about when to return Either and when to raise an error in Programmers.SE, I'm not qualified to answer it definitively.
• CmdrMoozy: Thanks for the feedback, guys. :) For the record, I asked a more in-depth question purely about error handling here: programmers.stackexchange.com/questions/252977/…