Change/increment the value of a while loop condition inside method - good or bad -

Viewed 878

I'm sorry if similar questions has been asked before, I couldn't find satisfactory answers by search.

The essence of the question is if its bad practice to "hide" increments/changes in a value used as a condition in a while loop.

int i = 0; // global/class variable

while(i < str.length()) {
    someMethod(i);
}

Where someMethod increments i.

I'm scanning some strings and based on character(s) I call different methods. The method gets the current index of the string as an argument, reads/process some characters, creates an object, and returns the new current index.

Example:

int i = 0;

while(i < str.length()) {
    if(condition(str.charAt(i))) i = someMethod(i, str);
    else if(condition2(str.charAt(i))) i = someOtherMethod(i, str)
    else ...
}

Things are however not that simple. In some cases the condition() method has to process some chars before it can conclude with true or false. If true we already know enough to conclude with which object to create. someMethod becomes more or less obsolete. We can just create the object in condition() instead, but we need to tell the loop the new value of i.

A solution is to declare i as a global/class variable, and set the correct value in condition(). However then we "hide" the change of the value i.

A possible solution is to have the method itself as a part of the condition.

int i = 0; 
int j = 0;

while(i < str.length()) {
    if(i < (j = someMethod(i, str))) i = j; 
    else if(i < (j = someOtherMethod(i, str))) i = j; 
    else..
}

If the correct sequence of chars is not present someMethod returns i unchanged (i == j, and the next if conditions executes). If the correct sequence is present, someMethod return i + the number of chars it used (i < j, and we get to i = j).

I don't like to have values "magically" change inside a method when its used as a condition in a loop, but I'm not a fan of the if(i < (j = someMethod(i, str))) either.

I'm a student and I just want your opinion about how you would write/solve this, in the most readable/maintainable way?

1 Answers

Using a mutable global variable int is probably not a good solution : it can be used anywhere by error in the class and so defeats the actual logic.
The way relying on an intermediary local variable makes the reading more complex for not good reasons.

So the first way where you assign the return of the invocation to i sounds better :

int i = 0;

while(i < str.length()) {
    if(condition(str.charAt(i))) i = someMethod(i, str);
    else if(condition2(str.charAt(i))) i = someOtherMethod(i, str)
    else ...
}

As alternative you could use a mutable wrapper class representing the index.
It avoids returning the new index as it is mutable and so you don't need to reassign it in the caller method.

You could also use AtomicInteger but it may seem a little to be a convoluted use.

public class Index {

    private int value;

    public void set(int value) {
        this.value = value;
    }

    public int get() {
        return value;
    }    
}

And use it :

String str = ...;

for(Index index = new Index(); index.get() < str.length();) {       
    if(condition(str.charAt(index.get())))  someMethod(index, str);
    else if(condition2(str.charAt(index.get()))) someOtherMethod(index, str);
    else ...
}
Related