I’ve tried to solve a challenge posted on a LinkedIn forum using Haskell – a language I’m still learning the basics of – and, while the code works correctly, I would like to get some feedback on the coding style, and learn what could be improved.

The task is to add all Armstrong numbers within a range.
An Armstrong (or narcissistic) number is a number that is equal to the sum of all its digits, each raised to the power of the length of its digits.
For instance, 153 is an Armstrong number because 13+53+33

$1^3+5^3+3^3$

$1^3 + 5^3 + 3^3$ equals 153.
The range can be given in any order – that is 5 10 and 10 5 denote the same interval, that is, all numbers between 5 and 10 (boundaries included).

And here’s my code:

digits :: (Integral n) => n -> [n]
digits n
  | n < 10 = [n]
  | otherwise = digits (n `div` 10) ++ [n `mod` 10]

numberLength :: Int -> Int
numberLength = length . digits

powerNumber :: Int -> [Int]
powerNumber n = map (d -> (d ^ exponent)) listOfDigits
    where exponent = numberLength n
          listOfDigits = digits n
 
isArmstrong :: Int -> Bool
isArmstrong a = a == sum (powerNumber a)
 
sortEnds :: Int -> Int -> [Int]
sortEnds a b = if a < b then [a, b] else [b, a]
 
sumAllArmstrongNumber :: Int -> Int -> Int
sumAllArmstrongNumber a b = sum([x | x <- [start..end], isArmstrong x])
    where [start, end] = sortEnds a b

Any feedback is much appreciated!

I would like to get some feedback on the coding style, and learn what could be improved.

First of all, it’s fantastic that all your functions have proper types, consistent indentation and style. Keep that in your future code style!

Prefer divMod or quotRem

If you use both y `div` x and y `mod` x for the same x and y then you should use divMod instead. Even better, use quotRem if possible. quotRem returns the same results for positive numbers but is slightly faster:

digits :: (Integral n) => n -> [n]
digits n
  | n < 10    = [n]
  | otherwise = let (q, r) = n `quotRem` 10
                in digits q ++ [r]

Cons; don’t append

There’s one big drawback in the new digits function though: we’re always appending a single element. This yields an O(n2)

$\mathcal{O}(n^2)$

$mathcal O(n^2)$ algorithm, since (x:xs) ++ [y] = x : (xs ++ [y]). Instead, we should cons the new element on the rest of the list:

digits :: (Integral n) => n -> [n]
digits n
  | n < 10    = [n]
  | otherwise = let (q, r) = n `quotRem` 10
                in r : digits q

Note that this will return the digits in reverse order but that’s fine. If you want the digits in the original order, consider using reverse on the result:

digits :: (Integral n) => n -> [n]
digits = reverse . go
    where go n = case n `quotRem` 10 of
                    (0, r) -> [r]
                    (q, r) ->  r : go q

I’m a fan of case for this style of quotRem usage, but that’s just my personal opinion.

Map vs pointfree vs comprehension

Warning: this section and mostly about personal preference.

In the following code, listOfDigits and expontent are defined and used once:

powerNumber :: Int -> [Int]
powerNumber n = map (d -> (d ^ exponent)) listOfDigits
    where exponent = numberLength n
          listOfDigits = digits n

I’d personally prefer digits n instead of listOfDigits, since it’s only used as an argument for map:

powerNumber n = map (d -> (d ^ exponent)) $ digits n
    where exponent = numberLength n

Next (d -> (d ^ exponent)) is (^exponent), which is preferred by some:

powerNumber n = map (^exponent) $ digits n
    where exponent = numberLength n

But in terms of readability, a list comprehension might even better:

powerNumber :: Int -> [Int]
powerNumber n = [ d ^ exponent | d <- digits n ]
    where exponent = numberLength n

Add additional information in return types

sortEnds :: Int -> Int -> [Int]
sortEnds a b = if a < b then [a, b] else [b, a]

The return type is slightly misleading: sortEnds always returns exactly two elements. So we should use a pair:

sortEnds :: Int -> Int -> (Int, Int)
sortEnds a b = if a < b then (a, b) else (b, a)

Remove superfluous parentheses

sumAllArmstrongNumber :: Int -> Int -> Int
sumAllArmstrongNumber a b = sum([x | x <- [start..end], isArmstrong x])
    where [start, end] = sortEnds a b

I’d remove the explicit parentheses around sum‘s argument:

sumAllArmstrongNumber :: Int -> Int -> Int
sumAllArmstrongNumber a b = sum [x | x <- [start..end], isArmstrong x]
    where (start, end) = sortEnds a b