Data type conversions are one of those areas where we have rich, well-supported, well-documented features built into most languages. Thus, we also have endless attempts for people to re-implement them. Or worse, wrap a built-in method in a way which makes everything less clear.

Mindy encountered this.

/* For converting (KEY_MSG_INPUT) to int format. */
public static int numberToIntFormat(String value) {
  int returnValue = -1;    	
  if (!StringUtil.isNullOrEmpty(value)) {
    try {
      int temp = Integer.parseInt(value);
      if (temp > 0) {
        returnValue = temp;
      }
    } catch (NumberFormatException e) {}
  }    	
  return returnValue;
}

The isNullOrEmpty check is arguably pointless, here. Any invalid input string, including null or empty ones, would cause parseInt to throw a NumberFormatException, which we’re already catching. Of course, we’re catching and ignoring it.

That’s assuming that StringUtil.isNullOrEmpty does what we think it does, since while there are third party Java libraries that offer that functionality, it’s not a built-in class (and do we really think the culprit here was using libraries?). Who knows what it actually does.

And, another useful highlight: note how we check if (temp > 0)? Well, this is a problem. Not only does the downstream code handle negative numbers, –1 is a perfectly reasonable value, which means when this method takes -10 and returns -1, what it’s actually done is passed incorrect but valid data back up the chain. And since any errors were swallowed, no one knows if this was intentional or not.

This method wasn’t called in any context relating to KEY_MSG_INPUT, but it was called everywhere, as it’s one of those utility methods that finds new uses any time someone wants to convert a string into a number. Due to its use in pretty much every module, fixing this is considered a "high risk" change, and has been scheduled for sometime in the 2020s.

[Advertisement] ProGet’s got you covered with security and access controls on your NuGet feeds. Learn more.