4
\$\begingroup\$

The question is that in a given integer n you have to sum its digits. For example, the sum for 111 would be 3.

Though I managed to solve the problem, I have a good feeling that this algorithm can still be improved.

public static int digit(int n){
    String numbers = n+"";
    int sum = 0;
    for(int x = 0; x < numbers.length(); x++){
        sum += Integer.parseInt(numbers.charAt(x)+"");
    }

    return sum;
}
\$\endgroup\$
7
\$\begingroup\$

There is an arithmetic solution to this problem, without converting to a string. It's probably what the string conversion does internally and will therefore likely be much more efficient.

Quoting Wikipedia:

The digit sum of a number \$x\$ in base \$b\$ is given by $$ \sum_{n=0}^{\lfloor \log_b x\rfloor} \frac{1}{b^n}(x \bmod b^{n + 1} - x \bmod b^n). $$

This StackOverflow answer has an implementation in C# (which is very similar to Java):

sum = 0;
while (n != 0) {
    sum += n % 10;
    n /= 10;
}

In addition, I'd like to comment on the naming of your method. In my opinion, digit doesn't describe its function very well. Instead I'd recommend something similar to digitSum.

Note that this approach doesn't handle negative numbers. They cause the function to go into an infinite loop. To handle that case, you can something like the following:

public static int digitSum(int n) {
    int sum = 0;
    bool isNegative = false;

    if (n < 0) {
        isNegative = true;
        n *= -1;
    }

    while (n != 0) {
        sum += n % 10;
        n /= 10;
    }

    if (isNegative) {
        return -sum;
    } else {
        return sum;
    }
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Not that it matters since int s do not have too much digits, but you could write more efficiently:

public static int digitSumV2(int n) {
    int sum = 0;
    String s = Integer.toString(n);

    for (int i = 0; i != s.length(); ++i) {
        sum += s.charAt(i) - '0';
    }

    return sum;
}

The problem in your implementation is that you keep converting each character to a string and then parse it; and so for each character of the text representation of the input number.

Hope that helps.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$
public static int digit(int n){

Methods do things, so it's often clearer if they get verb names like sum or sumDigits. A noun name like digit would make more sense for a variable or class name.

    for(int x = 0; x < numbers.length(); x++){
        sum += Integer.parseInt(numbers.charAt(x)+"");
    }

Consider using the range based version of the for loop.

    char [] digits = numbers.toCharArray();
    for (char digit : digits) {
        sum += digit - '0';
    }

This also uses the more direct character conversion rather than converting the character to a string to an integer.

This probably won't be faster than your version (although fewer conversions might help), but it's shorter and easier to read.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Explanation of my downvote: I don't think this answer adds anything to the question as both pieces of advice have already been said in other answers. \$\endgroup\$ – jacwah Nov 5 '16 at 17:17
  • \$\begingroup\$ @jacwah Please point to where in the other answers are my two pieces of advice: 1. Name methods as a verb like sumDigit not a noun like digitSum; 2. Use a range-based for loop instead an iteration variable to index an array. \$\endgroup\$ – mdfst13 Nov 5 '16 at 18:59
  • \$\begingroup\$ I also think using the foreach loop when you can is an important point. Note that Character.getNumericValue exists although whether it's clearer than digit - '0' may be debatable. \$\endgroup\$ – JollyJoker Nov 7 '16 at 10:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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