Function to sum all Armstrong numbers within a range

Question

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 \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!

Answer 1

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 \$\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

• Jir: That's just amazing feedback! Thank you so very much! I definitely see some habits from Python (where I come from), that is, the use of parentheses and lambda functions... very much like C-people add a semicolon in Python. I can also see where I'm "hacking" my way through the answer, without really knowing the implications of my choices (the "map vs pointfree vs comprehension" really got me thinking) or avoiding constructs I find unnatural (let ... in). Really golden feedback - I appreciate you spending your time on this!
• Zeta: @Jir there's nothing wrong with using where instead of let ... in. I happen to write a lot of (Emacs) Lisp lately, so (let (...) ...) was the first thing that came to my mind 😅