CodeSOD: A Percentage of Refactoring

Joseph was doing a refactoring effort, merging some duplicated functions into one, cleaning up unused Java code that really should have been deleted ages ago, and so on. But buried in that pile of code that needed cleaning up, Joseph found this little bit of code, to validate that an input was a percentage.

@Override
public Integer validatePercent(final String perc, final int currentPerc){
    char[] percProc= perc.toCharArray();
    char[] newPerc = new char[perc.length()];
    int percent=0;
    int y=0;
    if(percProc.length>4){
        return -1;
    }
    for(int x=0;x<percProc.length;x++){
        if(Character.isDigit(percProc[x])){
            newPerc[y]=percProc[x];
            y++;
        }
    }
    if(y==0){
        return -1;
    }
    
    String strPerc=(new String(newPerc));
    strPerc=strPerc.trim();
    if(strPerc.length()!=0){
        percent=Integer.parseInt(strPerc);
        if(percent<0){
            return -1;
        }else if(percent>100){
            return -1;
        }else if(Integer.parseInt(strPerc)==currentPerc){
            return -1;
        }else{
            return Integer.parseInt(strPerc);
        }
    }else{
        return-1;
    }
}

This validation function takes a string and an integer as an input, and immediately copies the string into an array, and makes a bonus array that's empty to start.

We reject strings longer than 4 characters. Then, we iterate over our input array and check each character; if that character is a digit, we copy it into the newPerc array, otherwise we… don't. If we copied at least one character this way, we continue- otherwise we reject the input.

Which right off the bat, this means that we accept 5, .05 and .0A5.

We take our newPerc array and turn it back into a string, trimming off any whitespace (which I'm fairly certain whitespace isn't a digit, last I checked, so there's nothing to trim).

If the string is greater than 0 characters, we parse it into an integer. If the result is less than zero, we reject it. Fun fact, isDigit also doesn't consider - a digit, so there's no chance we have a negative number here. If it's greater than 100 we reject it. If, when we parse it into an integer a second time, it's equal to the currentPerc input parameter, we also reject it. Otherwise, we return the result of parsing the string into an integer a third time.

So this isn't truly a validate function. It's a parse function. A strange one that doesn't work the way any sane person would want. And most annoying, at least in Java land, is that it handles errors by returning a -1, letting the caller check the return value and decide how to proceed, instead of throwing an exception.

Also, why reject the input if it equals the current value? I'd say that I'll puzzle over that, but the reality is that I won't. It's a stupid choice that I'd rather just not think more about.

[Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.

This post originally appeared on The Daily WTF.