I'll go through the code line-by-line:
1 getNthFromEnd :: Int -> [a] -> a
2 getNthFromEnd _  = error "Empty list given"
3 getNthFromEnd nth list
4 | listLength < nth = error $ "List length must be greater than " ++ show nth
5 | listLength == nth = head list
6 | listLength >= nth = list !! (listLength - nth - 1)
7 where listLength = length list
8 getNthFromEnd _ _ = error "Unknown exception occurred"
Type signature - good!
A function which calls out to
error is kinda frowned upon. Such functions are
called partial functions because they could throw an exception and that is not
indicated in the type signature. That's why some people don't like to use
tail - they prefer to use pattern matching instead for these functions.
This is not really a critic of your code, more just an observation. If you were
to write documentation for this function you might warn users that it is partial,
although they might be able to guess from the description that it necessarily has to be.
More on partial functions:
I would change the error message to
List length must be at least ... to match the comparison.
Lines 4 -- 6
Clearly one of the three comparisons has to hold, so for efficiency you can change
the guard in line 6 to an
| listLength < nth = ...
| listLength == nth = ...
| otherwise = ...
This avoids an unnecessary comparison check when
listLength is greater or euqal
Another common Haskell idiom is to match on the result of
compare like this:
-- may need Data.Ord
case compare listLength nth of
LT -> ...
EQ -> ...
GT -> ...
This has the advantage of being provably exhaustive by the compiler. However, it may turn out to be the slowest of the three. It depends a lot on how smart the compiler is. I don't see it used a lot in the Prelude functions for integer comparisons, and I imagine there is an efficiency reason behind that.
- The code you've written is exhaustive (handles all the cases) just because we know that one of the comparisons has to hold. But GHC can't prove this, and it may perform an extra comparison when it doesn't need to.
- Using an otherwise clause is exhaustive because we're using an
otherwise clause. But it avoids an extra check sometimes.
compare is provably exhaustive by the compiler.
In any case, this declaration will handle all the cases which leads to...
Based on the previous discussion, your function will never get to this point so
this line isn't needed. Even if it did, GHC will throw it's own error with a message
*** Exception: /tmp/err.hs:3:1-17: Non-exhaustive patterns in function foo
Moreover, having this catch-all clause is hiding a potentially useful warning. If you remove that line and compile with
-Wall GHC reports:
Pattern match(es) are non-exhaustive
In an equation for ‘getNthFromEnd’: Patterns not matched: _ (_ : _)
This raises two points:
The way you've written the code you are not proving to GHC that you are handling all of the list patterns. GHC would like you to write the second stanza with a pattern:
getNthFromEnd nth list@(:) =
That enables GHC to know that you've covered both the empty list and non-empty list cases.
- Using the catch-all clause prevents GHC from making this check.