Parsing words from string

Question

I am learning Java, so I am beginner. I know I can do this with split function but I want to improve my algorithm knowledge.

I will be glad if you help me. Thanks.

• inputStr variable is contains the string entered by the user.
• stopPos variable is contains the order of wanted the word.
• parseWith variable is contains which char to parse the string.

    public static String parseStr(String inputStr, int stopPos, char parseWith) {
        String toReturn = "";
        int unitCount = 0;

        for(int Iterator = 0; Iterator < inputStr.length(); Iterator = Iterator + 1){
            if (inputStr.charAt(Iterator) == parseWith){
                if (unitCount == stopPos) break;

                toReturn = "";
                continue;
            }
            if (toReturn == "") unitCount = unitCount + 1;
            toReturn += inputStr.charAt(Iterator);
        }

        try{
            return toReturn;
        } catch(Exception e){
            return null;
        }
    }

Here's an example of using:

public static void main(String[] args) {
    Scanner userEntry = new Scanner(System.in);
    System.out.print("Enter two numbers separated by space: ");
    String inputStr = userEntry.nextLine();

    int frstPart = Integer.parseInt(parseStr(inputStr, 1, ' '));
    int scndPart = Integer.parseInt(parseStr(inputStr, 2, ' '));
}

EDIT

I have changed some things from first part of code in here.

try{
    return toReturn;
} catch(Exception e){
    return null;
}

to

toReturn = (unitsCounter == stopPos) ? toReturn : null;
return toReturn;

Answer 1

• First of all, I see two problems with your choice of name for the int variable Iterator. For one thing, it goes against the Java coding convention of variable names starting with a lower case letter. And for another, and this might admittedly be more subjective than the first argument, there exists an interface named Iterator in the java.util package, and naming a primitive variable after an interface might be confusing to the reader, especially if the interface is from the package java.util which contains many commonly used classes and interfaces.

  Generally, with primitive variables like this one that only serve as a means to count loop iterations, it is common to only give them one-letter names, typically i or, in the case of nested loops, j etc., so unless you can think of a more descriptive or informative name, I suggest that you simply rename this variable to i.

• There is a shortcut for i = i + 1, which is ++i. In fact, there is also i++, which is subtly different from ++i, but this difference is not relevant for the two occurrences of this code pattern in your question (with Iterator and unitCount), so I won't explain this difference here but simply point out that you can replace both of the aforementioned occurrences with either the pre- or the post-increment expression, which is more concise and might therefore be easier to read.

• You are checking for reference equality with if (toReturn == "") when in fact you only want to compare the content of two String objects (if you are confused about what I mean, I found this article which might be helpful). So what you are actually interested in is if (toReturn.equals("")), or, even simpler in this special case, if (toReturn.isEmpty()).

• Next, about the structure of your code. You use break and continue, which means that the control flow of your program is not evident from its structure alone. This has the potential of making the code difficult to read, although this is not necessarily always true. But in your case, you have to read through the whole if-block containing the continue statement in order to find out under which conditions the subsequent two lines are executed. To avoid this complication, I would get rid of the continue statement and instead place the two lines after the large if block in an else block.

  While I cannot think of a better alternative to the break statement per se, the code might be easier to read if you put the remaining statement toReturn = ""; in an else block (the corresponding if block of which is the one that contains the break statement).

• This:

  toReturn = (unitsCounter == stopPos) ? toReturn : null;
  

  is more complicated than it needs to be because it explicitly instructs the compiler to assign toReturn to itself under specific circumstances, which is pointless and therefore confusing. Instead, you could just write this:

  if (unitsCounter != stopPos) {
      toReturn = null;
  }
  

• Strings are immutable, so every time you append a single character to the output string or clear the entire string, you are effectively creating a new object. Since you are doing this many times but only need to return an actual String object once, you would be better of with a StringBuilder for your internal appending and resetting operations.

• I think what also made the code a bit difficult to understand for me is your variable names, because I find some of them not really to the point. For example, toReturn is confusing because this string variable is frequently reset to 0 characters and populated anew before it is returned. A more fitting name would be currentUnit, in accordance with unitCount, in my opinion. parseWith is also not very descriptive, because the meaning of the preposition "with" seems to me to be ambiguous. An unambiguous alternative would be delimiter. I also found stopPos confusing, because "pos" sounds to me like a fixed-size unit, for example a character, but in your case, it concerns a variable-size unit (a sequence of characters). Maybe "segment" would be better, or "subsequence". Also, speaking of this variable, it would probably help if, whatever term you opt for, you also use this term in the variable currently named unitCount, for consistency.

Answer 2

Stingy makes many good points, though I'd disagree about calling any variable "i" - in my opinion you should always try to find a meaningful name for anything in your program.

In this case, I think the variable represents a character position in the string, so I'd probably call it something like "currentCharPos" (I'd even consider "currentCharPosition").

Building up Strings, even if you use StringBuilder, seems inelegant to me. If you find the start and end positions of "tokens" in the input String, you can use String.subString() to extract them much more simply.

If you're going to produce something that does this, it would be natural for it to implement the Iterable interface, I think.

Here's a simple example based on those ideas - note that my version assumes spaces as delimiters and assumes you can discard multiple spaces :

import java.util.Iterator;

public class StringSplit implements Iterator<String>, Iterable<String> {

  private final String toSplit;     // the string we're going to tokenize
  private int currentCharPos = 0;   // where we are in the string at any point in time
  String nextWord = null;           // the next token to be returned

  public StringSplit(String toSplit) {
    this.toSplit = toSplit;
    nextWord = getNextWord();       // the easiest approach is probably to pre-load the next token
  }

  private String getNextWord() {
    // find the first non-space
    while (currentCharPos < toSplit.length()) {
      if (toSplit.charAt(currentCharPos) != ' ') {
        break;
      }
      currentCharPos += 1;
    }

    if (currentCharPos >= toSplit.length()) { // end of string before we found one
      return null;
    }

    int startPos = currentCharPos;

    // Find next space (or end of string)
    while (currentCharPos < toSplit.length()) {
      if (toSplit.charAt(currentCharPos) == ' ') {
        break;
      }
      currentCharPos += 1;
    }

    return toSplit.substring(startPos, currentCharPos);
  }

  @Override  // From the Iterator interface
  public boolean hasNext() {
    return nextWord != null;
  }

  @Override  // From the Iterator interface
  public String next() {
    String result = nextWord;     // remember what we need to return
    nextWord = getNextWord();     // pre-load the next token
    return result;
  }

  @Override  // From the Iterable interface
  public Iterator<String> iterator() {
    return this;
  }

  // Do some basic testing
  public static void main(String[] args) {
    StringSplit splitter = new StringSplit("My hat it   has three corners");
    int wordNumber = 0;
    for (String word : splitter) {
      System.out.format("word %d is '%s'%n", wordNumber++, word);
    }
  }
}

Historically you might have used the StringTokenizer class for the purpose - the class is still around, and its source code is available, so you might find it instructive to review that.