CodeSOD: A Range of Skills

Ulvhamne works on a team with over a hundred other developers. It's a big group, working on a huge project. And some of the quality in that code base gets... variable. Worse, when a bug pops up, it can be tricky to even identify what in the code is triggering the bug, let alone what the root cause is.

For example, one of the config-file fields needed a number to specify the beginning and end of a range. If you put in a relatively short range- thousands or hundreds of values- everything worked fine. That was a pretty typical use case. But if you put in something closer to MAX_INT, everything worked fine for a little bit, but within moments the server would grind to a halt, memory would fill up, and the OS would hang as it ended up constantly thrashing pages to disk.

Ulvhamne joked with one of his co-workers. "Wouldn't it be funny if, instead of just tracking the range's endpoints, they populated an array with all the possible values instead?"

 public void addRange(int min, int max) { int key, value; for(int i = min; i <= max; ++i){ key = value = i; myMap.put(key, value); } } public void addValue(int value){ int key = value; myMap.put(key, value); } public boolean execute(int input){ Iterator<Integer> iter = myMap.values().iterator(); while(iter.hasNext()){ if(iter.next().intValue() == input) return true; } return false; }

As it turns out, that is almost exactly what they did. Ulvhamne was only wrong about it actually being a map. That this has an addRange and an addValue method also hints that they wrote a generic wrapper class around maps, and then reused that wrapper for any problem they could roughly hammer into the right shape to abuse the class. That it requires a per-element search to see if an item is contained in the range, at least as implemented here, is icing.

For bonus points: this was also within a 30,000 line "utility" file.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!