Haskell Integer Odd Digits Checker

Question

I seem to be stuck on a question and have no idea how to approach it or what Im doing wrong with my current code.

I have to write a function called oddDigits which takes a single integer argument and returns a boolean result. It should return True if and only if the argument is a positive integer with an odd number of digits. If the argument is zero or negative, the function should stop with an error message.

Also, cant convert the argument into a string. Have to use recursion. I have a feeling each digit could be stored in a list recursively and then the length of the list could determine the answer.

So far, I have this:

oddDigits :: Integer -> Bool 

lst = [] 

oddDigits x 
    | (x < 0) || (x == 0) = error 
    | x `mod` 10 ++ lst ++ oddDigits(x `div` 10) 
    | length(lst) `mod` 2 /= 0 = True
    | otherwise = False 

Sorry if the code looks horrible. I am new to Haskell and still learning. What exactly am I doing wrong and how could I correct it?

Answer 1

First off, this seems a pretty weird thing to check. Perhaps what you're doing wrong is to ever consider this problem...

But if you persist you want to know the property of an integer having an odd number of digits... oh well. There's a lot that could be improved. For starters, (x < 0) || (x == 0) doesn't need the parentheses – < and == (infix 4) bind more tightly than ||. If you're not sure about this, you can always ask GHCi:

Prelude> :i ==
class Eq a where
  (==) :: a -> a -> Bool
  ...
    -- Defined in ‘GHC.Classes’
infix 4 ==
Prelude> :i ||
(||) :: Bool -> Bool -> Bool    -- Defined in ‘GHC.Classes’
infixr 2 ||

But here you don't need || anyway because there's a dedicated operator for less-than-or-equal. Hence you can just write

oddDigits x 
  | x <= 0  = error "bla bla"
  | ...

Then, you can “convert” the number to a string. Converting to string is generally a really frowned-upon thing to do because it throws all structure, typechecking etc. out of the window; however “number of digits” basically is a property of a string (the decimal expansion), rather than a number itself, so this is not entirely unsensible for this specific task. This would work:

oddDigits x 
 | x <= 0                      = error "blearg"
 | length (show x)`mod`2 /= 0  = True
 | otherwise                   = False 

however it's a bit redundancy department redundant. You're checking if something is True, then give True as the result... why not just put it in one clause:

oddDigits x 
 | x <= 0     = error "blearg"
 | otherwise  = length (show x)`mod`2 /= 0

That's perhaps in fact the best implementation.

For any proper, sensible task, I would not recommend going the string route. Recursion is better. Here's what it could look like:

oddDigits 1 = True
oddDigits x 
 | x <= 0     = error "blearg"
 | otherwise  = not . oddDigits $ x`div`10

Answer 2

There's nothing wrong with your general approach of converting to a list of digits, then finding the length of the list. Really where you went wrong is trying to cram everything into one function. As you found out first hand, it makes it very difficult to debug. Functional programming works best with very small functions.

If you separate out the responsibility of converting an integer to a list of digits, using a digs function like the one from this answer, the rest of your algorithm simplifies to:

oddDigits x | x <= 0 = error
oddDigits x = odd . length $ digs x