Token iterator class to decompile math expressions not working with numbers

Question

I have a class where I have to print each token of a math expression. I have got my code to the point where it prints out everything except the numbers together. I am not sure what to fix in order to do that.

This is what the desired output is:

next token: [15] 

next token: [*] 

next token: [(] 

next token: [26] 

And this is what my code currently produces:

next token: [1] 

next token: [5] 

next token: [*] 

next token: [(] 

next token: [2] 

etc...

Here is my code:

public class TokenIter implements Iterator<String> {
    static int counter = 0;
    private String line;
    private String nextToken;

    public TokenIter(String line) {
        this.line = line;
    }

    public void remove() {
        throw new UnsupportedOperationException();
    }

    public static void main(String[] args) {
        String line = "  15*(26+37) - 443/7*612 - (233-543234)   ";
        System.out.println("line: [" + line + "]");
        TokenIter tokIt = new TokenIter(line);
        // print tokens in square brackets to check absence of white space
        while (tokIt.hasNext()) {
            if (tokIt.next() != "no")
                System.out.println("next token: [" + tokIt.next() + "]");
            counter++;
        }
    }

    public boolean hasNext() {
        if (counter < line.length())
            return true;
        else
            return false;
    }

    public String next() {
        String s = "";
        if (!hasNext())
            return null;
        else {
            char c = line.charAt(counter);
            if (c == ('/')) {
                counter = counter++;
                s = "" + c;
                return s;
            }
            if (c == ('+')) {
                counter = counter++;
                s = "" + c;
                return s;
            }
            if (c == ('*')) {
                counter = counter++;
                s = "" + c;
                return s;
            }
            if (c == ('-')) {
                counter = counter++;
                s = "" + c;
                return s;
            }
            if (c == ('(')) {
                counter = counter++;
                s = "" + c;
                return s;
            }
            if (c == (')')) {
                counter = counter++;
                s = "" + c;
                return s;
            } else if (c == ' ') {
                counter = counter++;
                return "no";
            }
            while (Character.isDigit(c)) {
                counter = counter++;
                s += "" + c;
                return s;
            }
        }
        return s;
    }
}

I understand there are other ways to do this like a string tokenizer with delimiters, but I need to do it this way.

Answer 1

Well, your code contains several issues:

• if (tokIt.next() != "no")

  This is not the proper way to compare Strings. Use #equals instead: if (!tokIt.next().equals("no")). Your current correctly compares your strings an can find "no", because you used a String literal. This wouldn't work if for example a user enters "no" into the console and you try to compare it using != "no". So keep that in mind

• You're calling tokIt.next() two times. The first time is in the if statement I've mentioned above and the second time is in System.out.println("next token: [" + tokIt.next() + "]");. If your code would move the cursor correctly (by increasing counter), then you would skip every second token. Create a new String to carry the token String token = tokIt.next(); and use it to compare it with "no" and to print it.

• You shouldn't call counter++; in your main method. This will move the cursor to the next character in the String. Leave that to the next method. Only this method should move the cursor.

• Not an issue, but your hasNext can be simplified to:

  public boolean hasNext() {
      return counter < line.length();
  }
  

• Don't use counter = counter++;. counter++ is the same as:

  int temp = counter;
  counter = counter + 1;
  return temp;
  

  So if you write counter = counter++; you're setting counter to its old value, after adding 1 to it. That means, counter wouldn't change its value. You could remove the counter = part, but ...

• Don't call counter++ in every if branch in your next method. Do it once in char c = line.charAt(counter);:

  char c = line.charAt(counter++);
  

  That way you read the value at the current counter and increase its value, so the next time you read something, it will be the next position.

• Don't create the variable s: This is not an issue, but it isn't helpful either. Since every if branch contains something like

  if (c == (...)) {
      s = "" + c;
      return s;
  }
  

  you can write instead:

  if (c == (...)) {
      return Character.toString(c);
  }
  

  This also avoid the "awkward" version of Character conversion to String: "" + c.

• Don't use these many if statements to do a simple check and then return the same character. You can do something like this instead:

  if ("/+*-()".indexOf(c) >= 0) {
      return Character.toString(c);
  }
  

  That way, you can check all allowed mathematical characters in one if statement. And it is "nicer" to add new characters.

• This is your current while loop which should read every combined digit:

  while (Character.isDigit(c)) {
      counter = counter++;
      s += "" + c;
      return s;
  }
  

  As you can see, it will return s on the first while loop iteration. What you really need is to remove that return and to read the next character from the source string to add it to s. This can look like this:

  String s = ""; // since we removed the old "s" (see my seventh remark), we need to create it here
  while (Character.isDigit(c)) {
      s += Character.toString(c); // add current character to String s
      c = line.charAt(counter++); // read next character
      if (!Character.isDigit(c)) { // if it is not a digit
          counter--; // move the cursor one position back
          break; // stop the loop
      }
  }
  

After you've followed my remarks, your output should then be:

line: [  15*(26+37) - 443/7*612 - (233-543234)   ]
next token: [15]
next token: [*]
next token: [(]
next token: [26]
next token: [+]
next token: [37]
next token: [)]
next token: [-]
next token: [443]
next token: [/]
next token: [7]
next token: [*]
next token: [612]
next token: [-]
next token: [(]
next token: [233]
next token: [-]
next token: [543234]
next token: [)]

I know, there are better ways to achieve OPs goal, but I think he still should use his own approach and learn from it.