CodeSOD: A Match Made In…

Andy C writes:

One of our colleagues dug this code up from an outsourced project. Took a few us to try to find out what it actually does, we're still not completely sure.

This is the associated Java code:

if (productList != null && !productList.isEmpty()) { for (int i = 0; i < productList.size(); i++) { String currentProductID = String.valueOf(productList.get(i).getProductId()); String toMatchProductID = String.valueOf(currentProductID); if (currentProductID.equals(toMatchProductID)) { productName = productList.get(i).getProductName(); break; } }}

If you just skim over the code, something you might do if you were just going through a large codebase, it looks like a reasonable "search" method. Find the object with the matching ID. But that's only on a skim. If you actually read the code, well...

First, we start with a check- make sure we actually have a productList. The null check is reasonable (I'll assume this predates Java's Optional type), but the isEmpty check is arguably superfluous, since we enter a for-loop based on size()- an empty list would just bypass the for loop. Still, that's all harmless.

In the loop, we grab the current item (item 0, on the first iteration), and that's our currentProductID. We choose to cast it into a string, which may or may not be a reasonable choice, depending on how we represent IDs. Since this is imitating a search method, we also need a toMatchProductID... which we make by cloning the currentProductID.

If the currentProductID equals the toMatchProductID, which it definitely will, we'll fetch the product name and then exit the loop.

So, what this method actually does is pretty simple: it gets the productName of the first item in the productList, if there are any items in that productList. The real question is: how did this happen? Was this a case of copy/paste coding gone wrong? Purposeful obfuscation by the outsourcing team? Just a complete misunderstanding of the requirements corrected through quick hacking without actually fixing the code? Some combination of all three?

We know what the code does. What the people writing it do, that we're definitely not sure about.

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