3
\$\begingroup\$

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?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ return num == 0 ? 0 : (num - 1) % 9 + 1 \$\endgroup\$
    – ielyamani
    Commented May 5 at 19:23
  • 1
    \$\begingroup\$ @ielyamani: Once again I suggest to write an answer :) \$\endgroup\$
    – Martin R
    Commented May 6 at 4:30

2 Answers 2

3
\$\begingroup\$

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
}
\$\endgroup\$
1
  • \$\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\$ Commented May 6 at 11:54
3
\$\begingroup\$

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:

  1. Document that caller must supply a non-negative input.
  2. 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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.