CODEWITHSUNDEEP
java
BubbleTree
BubbleTree

Reputation: 606

For loop doesn't increase counter

static void nodes(String node) {
  int node_location;
  int i;
  int update_i=1;
  node_location=((node.indexOf("(("))-2);
  ArrayList<String> node_array = new ArrayList<String>();

  for( i=1;i<node_location;i++) {
    if(node.charAt(i)!=',') {
      if(node.charAt(i+1)==',' || node.charAt(i+1)==')')
        node_array.add(Character.toString(node.charAt(i)));
      else {
        for(int a=i+2;a<=node.indexOf("),");a++) {
          update_i++;
          if(node.charAt(a)==',') {
            node_array.add(node.substring(i, a));
            break;
          }
        }
        i=update_i;
      }
    }
  }
}

This method is supposed to take a string in the format of (1,2,3,4,5) and store the numbers (as strings) in an arraylist. The problem is when I have (1,2,333,4,5) for example my if statement should go to else and check how many places the digit is before we reach a comma. Then I take the substring of that and store it into my arraylist. The problem is that for some reason the location of I does not update when we get to our else statement (we have to increment more than what the for loop for I does because the digit was more than one place. However when I run it, my program prints out the following:

1
2
333
333
33
3
4
5

Upvotes: 0

Views: 1299

Answers (5)

brimborium
brimborium

Reputation: 9512

As your question already has been "skeeted", I just have the following remark:

I think you are far better off with regex in this example. Here is code that splits your example string:

import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Main {
    public static void main(String[] args) {
        String str = "(3,4,555,6,4)";

        Pattern pattern = Pattern.compile("\\d+");
        Matcher matcher = pattern.matcher(str);

        while (matcher.find()) {
            System.out.println("found: " + matcher.group());
        }
    }
}

output:

found: 3
found: 4
found: 555
found: 6
found: 4

Explanation: \d (\\d in Java to escape the '\') is the shorthand character class for a digit. The + means "previous term once or more". The + is greedy, so it takes as many of the digits into one match as possible. The rest of the code is just Java syntax for regex matching.

Upvotes: 2

Jonathan Drapeau
Jonathan Drapeau

Reputation: 2610

For what I understand and how you want to do it, there's a few mistakes in your code.

Starting with your node_location variable that is not initialized correctly, the code you provided makes sure the first loop is not even entered.

Your first loop starts at 1 while indexes of String are 0 based, meaning you're not checking the first character of the node. That loop also ends when i reaches node_location, but, if you want all the numbers between parenthesis in node, it should stop when reaching the closing one. So node_location should be initialized with the index of ) in node. This is assuming you only have 1 closing parenthesis.

Before your second loop, you should be initializing update_1 with i or you'll update i, after the loop, with 1 + the number of loop done in your else.

Your second loop is not correct either. What you want to do is lopp until you reach a coma or a closing parenthesis. The way you coded the loop for the String you're expecting, as Jon Skeet pointed out, it will return -1 and won't enter the loop.

You should have the same condition to check if you're done with your multiple characters number as you got before if (node.charAt(i + 1) == ',' || node.charAt(i + 1) == ')') {. You'll need a variable (either String or StringBuilder) to gather all the characters forming your number so you can add it, once you have them all, to node_array. That loop should also ends, if the if is not matched, when it reached node_location.

As many mentionned, there's better, simpler solutions to your needs but what I provided here is a good start to improve your own code.

Upvotes: 0

Abubakkar
Abubakkar

Reputation: 15644

I think you should use split method of String class as it will be much easier :

static void nodes(String node)
{

   ArrayList<String> node_array = new ArrayList<String>();
   String allValues[] = node.split(",");
   for(String value : allValues){
      node_array.add(value);
   }
}

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1500055

It's hard to understand both your code and your question, unfortunately - but this looks relevant:

for(int a=i+2;a<=node.indexOf("),");a++)

Given that your original string doesn't contain ")," at all, indexOf will return -1, and you'll never go into your loop body. Therefore update_i will never be incremented, and you'll set i back to 1.

I would strongly advise you to completely rewrite your code - at the moment it looks way too complicated for what it's achieving. Can't you split on , and then remove any non-digit characters from each string?

Upvotes: 7

NPE
NPE

Reputation: 500247

I think one of the problems is that you only initialize update_i at the top, instead of doing it every time you enter the else branch.

P.S. Why all the complexity and not just:

String[] tok = node.split(",");

having removed the parentheses first?

Upvotes: 2

Related Questions