5
\$\begingroup\$

The problem is stated as follows:

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. (Note: Character.isDigit(char) tests if a char is one of the chars '0', '1', .. '9'. Integer.parseInt(string) converts a string to an int.)

sumNumbers("abc123xyz") → 123

sumNumbers("aa11b33") → 44

sumNumbers("7 11") → 18

Here's the program I wrote based on Character.isDigit(char) and Integer.parseInt(string).

public int sumNumbers(String str) {
  int sum = 0;
  for (int i = 0; i < str.length(); i++) {
      if (Character.isDigit(str.charAt(i))) {
          int count = 0;
          for (int j = i; j < str.length(); j++) {
              if (Character.isDigit(str.charAt(j))) count++;
              else break;
          }
          sum += Integer.parseInt(str.substring(i, i + count));
          i += count;
      }
  }
  return sum;
}

I'm not sure if I should make it more modular since it's pretty straightforward. I was wondering if I could do this without count. Also, I know it's nit-picky, but I want to know if the way I format my for loops and if else statements is good style. This is just a standalone program, so I'm not really concerned about the public/private issue.

\$\endgroup\$

4 Answers 4

4
\$\begingroup\$

I want to know if the way I format my for loops and if else statements is good style.

On the whole, yeah, looks good.

I recommend you use braces for one-liner blocks as well. In this instance, I'm not batting an eye at it—it's fine—but it's a good habit to learn.

I was wondering if I could do this without count.

Yes, since count = j - i, you can elide it.

int sumNumbers(String str) {
  int sum = 0;
  for (int i = 0; i < str.length(); i++) {
    if (Character.isDigit(str.charAt(i))) {
      int j;
      // start at i+1 because we know i has a digit
      for (j = i + 1; j < str.length(); j++) {
        if (!Character.isDigit(str.charAt(j))) {
          break;
        }
      }
      sum += Integer.parseInt(str.substring(i, j));
      i = j;
    }
  }
  return sum;
}

Alternatively, if the break is a bit of an eye-sore, you could hoist the check into the for-header, but I'm not 100% it's an improvement:

int sumNumbers(String str) {
  int sum = 0;
  for (int i = 0; i < str.length(); i++) {
    if (Character.isDigit(str.charAt(i))) {
      int j;
      // start at i+1 because we know i has a digit
      for (j = i + 1; j < str.length() && Character.isDigit(str.charAt(j)); j++);
      sum += Integer.parseInt(str.substring(i, j));
      i = j;
    }
  }
  return sum;
}

Note: String.substring creates a new string since Java 7—in the reference implementation, at least, and String.toCharArray creates a new object too. If you want to avoid these allocations, you can do the char-to-digit conversion manually, but it's a bit messier, easy to muck up, and I feel that's out of scope for the exercise.

\$\endgroup\$
2
  • \$\begingroup\$ I like you're answer the best because it answered my individual questions even though I think Oscar Smith had a great answer as well. \$\endgroup\$ Commented Oct 29, 2017 at 9:29
  • \$\begingroup\$ Also, the boolean condition in the second for loop is more of an eye sore to me. I find nothing wrong with break. \$\endgroup\$ Commented Oct 29, 2017 at 9:30
4
\$\begingroup\$

There are a couple things you can do here to make things simpler. The first is to make your inner loop a while loop instead of a for loop. With this idea, you can build up each number char by char, using Character.getNumericValue() which will return 1 for '1' etc. With charVal, we can incrementally build up our number by multiplying the by 10 and adding the new digit. When we reach a non-digit, we add the current num to our sum, and set sum to 0. Since the code now only operates one char at a time, we don't need i, or j either. With these changes, your updated code is

public int sumNumbers(String str) {
    int sum = 0;
    int num = 0;
    for (char ch : exampleString.toCharArray()) {
        int digit = Character.getNumericValue(ch);
        if (digit >= 0 && digit <= 9) {
            num = num * 10 + digit;
        } else {
            sum += num;
            num = 0;
        }
    }
    return sum + num;
}
\$\endgroup\$
9
  • \$\begingroup\$ How come you don’t initialize num to 1? And I think digit would be a better name. \$\endgroup\$ Commented Oct 28, 2017 at 18:49
  • 1
    \$\begingroup\$ You initialize num to 0, because the first time you see a 5 (eg), you want num*10 + 5 to equal 5, which means num needs to start at 0. It also means that when you don't hit number characters, you can add num to sum without changing the answer. \$\endgroup\$ Commented Oct 28, 2017 at 18:52
  • \$\begingroup\$ You also have some typos like missing ;'s and and an extra ) but when I started testing your program, I get different results. Some tests are now failing. sumNumbers("abc123xyz") returns 0 instead of 123. \$\endgroup\$ Commented Oct 28, 2017 at 19:00
  • \$\begingroup\$ I edited your code to fix the above test case but cases where the integer is the last value in the String such as sumNumbers("7 11") are returning the first integers like 7 but forgetting about the last one like 11. \$\endgroup\$ Commented Oct 28, 2017 at 19:16
  • 1
    \$\begingroup\$ I just fixed the sumNumbers("7 11") case \$\endgroup\$ Commented Oct 28, 2017 at 19:21
0
\$\begingroup\$

Your code looks fine and is short enough to be understood.

If you don't want to work on the level of individual characters, you can use regular expressions. Here is the code:

public static int sumNumbers(String str) {
    Matcher m = Pattern.compile("[0-9]+").matcher(str);
    int sum = 0;
    while (m.find()) {
        sum += Integer.parseInt(m.group());
    }
    return sum;
}

This is not the fastest code, but I find it easy to read once you know that [0-9] means a digit, and the + means 1 or more times.

\$\endgroup\$
-2
\$\begingroup\$

Here is my simple code for this one

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

  for(int i=0;i<str.length();i++){

    if(Character.isDigit(str.charAt(i))){
        int pos = i;
        while(pos < str.length() && Character.isDigit(str.charAt(pos))){
          pos++;

        }

         total += Integer.parseInt(str.substring(i,pos));  
          i = pos;
    }


  }

  return total;


}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! You have presented an alternative solution, but haven't reviewed the code. Please explain your reasoning (how your solution works and why it is better than the original) so that the author and other readers can learn from your thought process. Please read Why are alternative solutions not welcome? \$\endgroup\$ Commented Oct 30, 2019 at 17:36

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.