Article 4PJJX CodeSOD: UnINTentional Errors

CodeSOD: UnINTentional Errors

by
Remy Porter
from The Daily WTF on (#4PJJX)

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.

proget-icon.png [Advertisement] ProGet can centralize your organization's software applications and components to provide uniform access to developers and servers. Check it out! TheDailyWtf?d=yIl2AUoC8zAptwP0GhZQ8U
External Content
Source RSS or Atom Feed
Feed Location http://syndication.thedailywtf.com/TheDailyWtf
Feed Title The Daily WTF
Feed Link http://thedailywtf.com/
Reply 0 comments