Task: Write a function, which computes the Digital Root of a given number n.
Here's the solution, which I developed:
func computeSum(num: Int) -> Int {
let parts = "\(num)".split(separator: "")
return parts.reduce(into: 0) { accu, curr in
accu += Int(curr) ?? 0
}
}
func digitalRoot(of number: Int) -> Int {
let sum = computeSum(num: number)
return "\(sum)".count > 1 ? digitalRoot(of: sum) : sum
}
// Given examples
print(digitalRoot(of: 16)) // Should the 7
print(digitalRoot(of: 942)) // Should be 6
print(digitalRoot(of: 132189)) // Should be 6
print(digitalRoot(of: 493193)) // Should be 2
My implementation returns the expected values for the four given examples. So I guess it's formally correct.
What's your opinion about my solution concerning naming, style and algorithm? What could be possible improvements?
2 Answers 2
Your implementation of computeSum
converts the number to a string, then splits the string into an array of substrings (one for each digit), and finally converts the single-character substrings back to numbers in order to add them.
The same task can be achieved with integer operations, without any conversions to strings at all, just using integer division and the remainder operator. That is more efficient in terms of memory and time.
The verb "compute" in the function name computeSum
does not convey any information, I would call it digitSum(of:)
instead.
A "precondition" can be added to check for nonnegative arguments. In unchecked builds, the program is terminated if the conditions is not matched. This helps to find wrong uses of the function during development.
func digitSum(of number: Int) -> Int {
precondition(number >= 0, "number must be nonnegative")
var sum = 0
var n = number
while n > 0 {
sum += n % 10 // Add the last digit of `n` to `sum`
n /= 10 // ... and remove it from `n`.
}
return sum
}
The digitalRoot
function repeatedly computes the digit sum until the result has at most one digit. This can also be done with integer operations alone instead the string conversion "\(sum)".count > 1
.
Also no call to computeSum/digitSum
is needed at all if the given number is already less than 10:
func digitalRoot(of number: Int) -> Int {
precondition(number >= 0, "number must be nonnegative")
return number < 10 ? number : digitalRoot(of: digitSum(of: number))
}
Alternatively with a loop instead of (tail) recursion, that is a matter of personal preferences:
func digitalRoot(of number: Int) -> Int {
precondition(number >= 0, "number must be nonnegative")
var n = number
while n >= 10 {
n = digitSum(of: n)
}
return n
}
-
\$\begingroup\$ 'The verb "compute" in the function name computeSum does not convey any information'. Amen. My favorite rule of thumb for naming functions: "verbs are for side effects". \$\endgroup\$Joachim Lous– Joachim Lous2024年05月06日 11:54:30 +00:00Commented May 6, 2024 at 11:54
formally correct?
No.
accu += Int(curr) ?? 0
Most authors don't define Digital Root for negative integers, preferring to stick to the natural numbers, perhaps handling any negative signs on the outside before we examine a root.
Thank you for citing a reference, wikipedia. Let's consider digital root of e.g. ±7. Unsurprisingly for 7 we obtain 7. Plugging in values for -7, we obtain 2.
Given that you turned an integer into a string,
we know that curr
could only match a regex
of ^[0-9-]$
, that is, digits and "-"
minus sign.
The value of interest is Int("-") ?? 0
, or zero.
In other words, you're evaluating digital root
of the absolute value of num
,
leading to an incorrect root of 7 for input of -7.
I would prefer that you offer a more conservative implementation:
- Document that caller must supply a non-negative input.
- Raise fatal error if caller violates that spec. Either blow up immediately upon entry with a pre-condition check, or blow up organically when
Int("-")
balks at its input and caller gets what he deserves.
names
Your abbreviation for "current" is fine.
But accu
inconveniently has two syllables, and
most writers would choose an "accumulator" symbol of acc
.
Clearly showing that sum
is a distinct concept from digitalRoot
was very nice.
design of Public API
Your exposed surface area is small, just two functions, so a new developer doesn't have much to learn about calling into this library.
But consider marking the computeSum()
helper private
,
so a new developer will have even less to worry about.
test suite
possible improvements?
print(digitalRoot(of: 16)) // Should the 7
print(digitalRoot(of: 942)) // Should be 6
print(digitalRoot(of: 132189)) // Should be 6
print(digitalRoot(of: 493193)) // Should be 2
This should be a proper automated unit test, please. One which "knows the right answer" so it can display a Green bar, without having to manually eyeball the result to judge its correctness. You might use XCTestCase, or some similar framework.
corner cases
It is axiomatic that "a piece of code which hasn't executed yet
is probably buggy."
Your examples above never exercised the second
part of the Int(curr) ?? 0
expression.
It can be challenging to achieve full test coverage
of the target code, but it does pay rewards.
return num == 0 ? 0 : (num - 1) % 9 + 1
\$\endgroup\$