CodeSOD: A Trying Block

Mark sends us a very simple Java function which has the job of parsing an integer from a string. Now, you might say, “But Java has a built in for that, Integer.parseInt,” and have I got good news for you: they actually used it. It’s just everything else they did wrong.

<span class="hljs-keyword">private</span> <span class="hljs-type">int</span> <span class="hljs-title function_">makeInteger</span><span class="hljs-params">(String s)</span>
{
  <span class="hljs-type">int</span> i=<span class="hljs-number">0</span>;
  <span class="hljs-keyword">try</span>
  {
    Integer.parseInt(s);
  }
  <span class="hljs-keyword">catch</span> (NumberFormatException e)
  {
    i=<span class="hljs-number">0</span>;
    <span class="hljs-keyword">return</span> i;
  }
  i=Integer.parseInt(s);
  <span class="hljs-keyword">return</span> i;
}

This function is really the story of variable i, the most useless variable ever. It’s doing its best, but there’s just nothing for it to do here.

We start by setting i to zero. Then we attempt to parse the integer, and do nothing with the result. If it fails, we set i to zero again, just for fun, and then return i. Why not just return 0? Because then what would poor i get to do?

Assuming we didn’t throw an exception, we parse the input again, storing its result in i, and then return i. Again, we treat i like a child who wants to help paint the living room: we give it a dry brush and a section of wall we’re not planning to paint and let it go to town. Nothing it does matters, but it feels like a participant.

Now, Mark went ahead and refactored this function basically right away, into a more terse and clear version:

<span class="hljs-keyword">private</span> <span class="hljs-type">int</span> <span class="hljs-title function_">makeInteger</span><span class="hljs-params">(String s)</span>
{
  <span class="hljs-keyword">try</span>
  {
    <span class="hljs-keyword">return</span> Integer.parseInt(s);
  }
  <span class="hljs-keyword">catch</span> (NumberFormatException e)
  {
    <span class="hljs-keyword">return</span> <span class="hljs-number">0</span>;
  }
}

He went about his development work, and then a few days later came across makeInteger reverted back to its original version. For a moment, he wanted to be mad at someone for reverting his change, but no- this was in an entirely different class. With that information, Mark went and did a search for makeInteger in the code, only to find 39 copies of this function, with minor variations.

There are an unknown number of copies of the function where the name is slightly different than makeInteger, but a search for Integer.parseInt implies that there may be many more.

[Advertisement]
Keep all your packages and Docker containers in one place, scan for vulnerabilities, and control who can access different feeds. ProGet installs in minutes and has a powerful free version with a lot of great features that you can upgrade when ready.Learn more.

Source: Read More