CodeSOD: Concatenated Validation

User inputs are frequently incorrect, which is why we validate them. So, for example, if the user is allowed to enter an "asset ID" to perform some operation on it, we should verify that the asset ID exists before actually doing the operation.

Someone working with Capybara James almost got there. Almost.

private boolean isAssetIdMatching(String requestedAssetId, String databaseAssetId) {

    return (requestedAssetId + "").equals(databaseAssetId + "");

}

This Java code checks if the requestedAssetId, provided by the user, matches a databaseAssetId, fetched from the database. I don't fully understand how we get to this particular function. How is the databaseAssetId fetched? If the fetch were successful, how could it not match? I fear they may do this in a loop across all of the asset IDs in the database until they find a match, but I don't know that for sure, but the naming conventions hint at a WTF.

The weird thing here, though, is the choice to concatenate an empty string to every value. There's no logical reason to do this. It certainly won't change the equality check. I strongly suspect that the goal here was to protect against null values, but it doesn't work that way in Java. If the string variables are null, this will just throw an exception when you try and concatenate.

I strongly suspect the developer was more confident in JavaScript, where this pattern "works".

I don't understand why or how this function got here. I'm not the only one. James writes:

No clue what the original developers were intending with this. It sure was a shocker when we inherited a ton of code like this.

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