Infinite loop in StringUtil

Contribution Depot
1

Just spotted it today :slight_smile:

[java]
public String padNumZero(float num, int wholeLen, int decimalPlaces) {
// …

    dpLoc = numStr.indexOf(".");
    int length = numStr.substring(dpLoc).length();
    while (length < decimalPlaces) {//< ---------------------------------- FIXME: this definitely will never end ;)
        numStr += "0";
    }
    return numStr;
}

[/java]

Suggested fix:

[java]
public String padNumZero(float num, int wholeLen, int decimalPlaces) {
// …

    dpLoc = numStr.indexOf(".");
    while (numStr.substring(dpLoc).length() < decimalPlaces) {
        numStr += "0";
    }
    return numStr;
}

[/java]

2

Is that metod actualy used anywhere? Cause I would suspect not (as so far noone complained about it) Might even be a case for complete removal then?

3

My guess is that there is actually a missing length++. The anti-idiom here is using a while() loop instead of a for() loop when the latter is closer to intent.

…but anyway, I also wonder if it’s even used and can just be removed.

4

Also, repeatedly using String addition… copying String objects and with quadratic behaviour.
Just set up a StringBuffer and fill it with a for loop.

This code is a candidate for http://thedailywtf.com and should be rewritten from scratch if it is actually called from anywhere.

5

Actually modern compilers turn string concatenation into StringBuilder behind the scenes. I’m not sure just how smart they are about it but I think in that case they would catch it.

6

Yes, that’s how String concatenation is defined.
The OpenJDK compiler isn’t smart enough to hoist the StringBuilder outside the loop though, so the string data is still copied every time through the loop.

7

The method is never used indeed, only a subset of the class’ methods is used in the jme3tools.navigation package.
I think it’s a very old contribution about navigation (as in sailing). I don’t really know why it’s in the core, it would fit as a plugin IMO. But maybe at the time making it a plugin wasn’t an option.
Maybe @normen knows something about it.

IMO the whole package should be removed from core.

8
@nehon said: The method is never used indeed, only a subset of the class' methods is used in the jme3tools.navigation package. I think it's a very old contribution about navigation (as in sailing). I don't really know why it's in the core, it would fit as a plugin IMO. But maybe at the time making it a plugin wasn't an option. Maybe @normen knows something about it.

IMO the whole package should be removed from core.

you got it ^^

9

I think it is a good candidate for removal from 3.0.1 / 3.1