1
\$\begingroup\$

I am going through the CodingBat exercises for Java. Here is the task I have just completed:

Given a string, return the sum of the numbers appearing in the string, ignoring all other characters. A number is a series of 1 or more digit chars in a row.

And here is my code:

public int sumNumbers(String str){
    int total = 0;

    for (int i = 0; i < str.length(); i++) {
        if (Character.isDigit(str.charAt(i))) {
            StringBuilder appendNums = new StringBuilder();
            appendNums.append(str.charAt(i));
            for (int j = i+1; j < str.length(); j++) {
                if (Character.isDigit(str.charAt(j))) {
                    appendNums.append(str.charAt(j));
                } else {
                    break;
                }
            }
            String appendNums2String = appendNums.toString();
            total += Integer.parseInt(appendNums2String);
            i += appendNums2String.length()-1;
        }
    }
    return total;
}

The questions I have are:

  1. Are there any parts of this code you find to be poor practise or poor quality?
  2. Is it acceptable to append one digit before the (conditional) digits that proceed it, or should I be identifying the entire block first, and then append?
  3. What is a more concise way of solving this?
  4. For this question I relied heavily on Eclipse's debugger until I got it right. Is this good or bad practise?
  5. Is it a better idea to use a while loop instead of the second for loop, because the characters tested are relative to i rather than the string's length? (I played around with this but couldn't figure it out.)
\$\endgroup\$
3
\$\begingroup\$

Learning to use, and using the debugger are fantastic ideas. It is a valuable skill that will serve you well. Understanding how your code works (and breaks) is very valuable, and the debugger helps you through there.

The remainder of your specific questions are somewhat linked to your actual implementation, and I am going to suggest that your implementation would be simpler with a String split operation. The specification says:

Given a string, return the sum of the numbers appearing in the string, ignoring all other characters. A number is a series of 1 or more digit chars in a row.

Taking this literally, I would code this up as a Regular expresison system matching digits. Something like:

private static final Pattern NUMBERS = Pattern.compile("[0-9]+");

private static int digitSum(String s) {
    Matcher matcher = NUMBERS.matcher(s);
    int sum = 0;
    while (matcher.find()) {
        sum += Integer.parseInt(matcher.group());
    }
    return sum;
}

The Pattern says "Look in the string for a sequence of digits".

The method loops through all digit sequences, converts them to integers, and totals them.

\$\endgroup\$
  • \$\begingroup\$ This is a really clever solution. I enjoy practising with Pattern and Matcher (it's a shame CodingBat won't allow their usage) \$\endgroup\$ – alanbuchanan Apr 13 '15 at 17:58
2
\$\begingroup\$

I would advise using the Scanner class to do this for you.

private static int digitSum(String s) {
    Scanner sc = new Scanner(s);
    sc.useDelimiter("\\D+"); // read numbers separated by non-numerics
    int sum = 0;
    while (sc.hasNextInt()) {
        sum += sc.nextInt();
    }
    sc.close();
    return sum;
}

If the numbers are particularly large, you can switch the scanner methods to hasNextBigInteger and nextBigInteger.

As for your code, I have a few comments on your naming

  • appendNums doesn't sound like a thing (an object), it sounds more like an action (method). I would change this to appendedDigitsBuffer
  • numbers don't belong in any name. Spell out appendNums2String and change it to a non verb (as per previous reason) to appendedDigits
\$\endgroup\$

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.