Method to convert String to int array

Question

I was working on a suggestion for this question, but the method, I would've suggested the OP do something similar to, seems a bit convoluted.

It's supposed to take a String input and return an array of only the integers deliminated by spaces. It does this correctly, but It feels like I may be doing some unnecessary things or there's a more succinct way to do this, such as with regex.

public static int[] getIntegers(String s) {
 int[] result;
 ArrayList<Integer> helper = new ArrayList<>();
 StringBuilder sb = new StringBuilder();
 for (int i = 0; i < s.length(); i++) {
 if (s.charAt(i) == ' ' ^ i == s.length() - 1) {
 if (i == s.length() - 1) { sb.append(s.charAt(i)); }
 if (sb.toString().length() > 0) {
 try { helper.add(Integer.parseInt(sb.toString()));
 } catch(NumberFormatException nfe) {
 // Ignore non-integers
 }
 sb.setLength(0);
 continue;
 }
 }
 sb.append(s.charAt(i));
 }
 result = new int[helper.size()];
 int i = 0;
 for (Integer n : helper) {
 result[i++] = n;
 }
 return result;
}

• Legato: Thinking this may be more appropriate for StackOverflow.
• h.j.k.: Would it be appropriate to tag this with rags-to-riches too? Not sure the extent of the makeover here. :D
• Legato: I personally wouldn't be opposed. Not just one but two superior methods!

Answer 1

Ohhhh.... XOR logic on booleans in Java ..... how esoteric:

if (s.charAt(i) == ' ' ^ i == s.length() - 1) {

That should be removed "just because". It's hard to understand, the logic is inverted, and it is not short-circuit, so it tests the i == s.length() - 1 even when s.charAt(i) == ' '

I think what you have is an inverted logic problem. You are looking for all the digits in the input, when, instead, you should be looking for not-digits....

The supporting logic, using StringBuilders, etc. is not a bad idea.... but, have you considered a regular-expression?

String[] digitwords = input.split("\\D+");
int[] result = new int[digitwords.length];
for (int i = 0; i < result.length; i++) {
 result[i] = Integer.parseInt(digitwords[i]);
}
return result;

The split there, is "split on all sequences that do not contain digits"

Java8 may help (with an added filter to remove empty values, not in the loop above), with:

return Arrays.stream(input.split("\\D+"))
 .filter(word -> !word.isEmpty())
 .mapToInt(Integer::parseInt)
 .toArray();

• Legato: Could you elaborate on what you mean by "short-circuit?" Isn't testing both i == s.length() - 1 and s.charAt(i) == ' ' the point? Is XOR really that uncommon? Thanks for the fantastic answer! It's exactly what I was looking for, I'm not too knowledgeable on using Regex, but I was sure there'd be a more eloquent way to do it with through that. The java 8 take is very well received. Both these methods get letters otherwise in between letters, possibly desirable behavior. Again, awesome answer!
• 200_success: Splitting on \D+ would obliterate negative signs. Splitting on whitespace is probably closer to the intended behaviour.
• alicederyn: You can simplify the third line of your nice Java 8 example to: .mapToInt(Integer::parseInt)
• rolfl: @chrispy - of course. It's a bad habit I got in to when I first started playing with Java8, and it's taking me a while to undo that.... in this case, I wrote that at my worst (but I still do it too often). It looks more natural to me for some reason to see the arguments passed to the function. Still, it's not right, you are.

Answer 2

Well, @rolfl's is nearly identical to mine, and it's a lot simpler.

If you wanted to use your approach, here's how I would tweak it:

public static int[] getIntegers(String s) {
 ArrayList<Integer> helper = new ArrayList<>();
 StringBuilder sb = new StringBuilder();
 for (int i = 0; i <= s.length(); i++) {
 if (i == s.length() || s.charAt(i) == ' ') { // End or delimiter
 try {
 helper.add(Integer.parseInt(sb.toString()));
 } catch (NumberFormatException discardNonIntegers) {
 }
 sb.setLength(0);
 } else { // Possible number
 sb.append(s.charAt(i));
 }
 }
 int[] result = new int[helper.size()];
 int i = 0;
 for (int n : helper) {
 result[i++] = n;
 }
 return result;
}

Changes to note:

• int[] result is declared where it is assigned.
• The loop terminates at a fictitious index just past the end of the string, to eliminate the special case if (i == s.length() - 1) { sb.append(s.charAt(i)); }
• if (i == s.length() || s.charAt(i) == ' ') is easier to understand than your XOR, I think.
• It is not necessary to check if (sb.toString().length() > 0), as trying to parse an integer from an empty string would just throw an exception anyway. If you did want to check, then if (sb.length() > 0) would work.
• You have to choose a name for NumberFormatException, so you might as well make it meaningful.
• I find if/else easier to understand than if-continue. Due to the parallelism in formatting, it's easier to see that on each iteration of the loop we are either going to clear the buffer or append a character.
• Nobody likes boxed types. You might as well unbox n slightly sooner.

• Legato: Yeah, come to think of it the continue was no longer necessary(I tried something else before and it was a remnant), Thank you for the 'kindred to mine' approach! It's insightful.