Article QJ4T CodeSOD: Validate My Feelings of Cleverness

CodeSOD: Validate My Feelings of Cleverness

by
Remy Porter
from The Daily WTF on (#QJ4T)

It's not uncommon to have a branch in your code along the lines of, if (debugMode) { doSomeLogTypeThingy(); }. Usually, we try and wrap that sort of stuff up and keep it far away from the actual business logic, and the result is a plethora of logging frameworks and diagnostic libraries.

They are, however, consistent about one thing: whether or not debugMode is enabled or not, the actual business logic should behave the same way. They're designed and intended to be minimally disruptive.

Remco found this method:

 public static void objectIsValid(final Object objectToCheck, final String objectName) { if(objectToCheck instanceof Validatable){ objectIsValid((Validatable)objectToCheck, objectName); }else{ if( log.isDebugEnabled() && objectToCheck != null && objectToCheck.getClass().getPackage().getName().startsWith("com.initrode") && !objectToCheck.getClass().getName().contains("Enum") ){ throw new IllegalArgumentException("this class should implement validatable:"+objectToCheck.getClass()); } validateObjectIsNotNull(objectToCheck, objectName); } }

This mess of code takes the interesting stance that, if debugging is enabled, that means it should throw exceptions, but if debug mode isn't enabled, it should just" not do anything?

Of course, that's not the only useless thing in that method. Note at the top, where they use instanceof to check if the object can be cast to a Validatable? And then it passes it to what is obviously an overloaded method that accepts a Validatable?

 public static void objectIsValid(final Validatable objectToCheck, final String objectName) { validateObjectIsNotNull(objectToCheck,objectName); ConstraintViolation[] invalidValues = validatorRepos.validate(objectToCheck); if(invalidValues.length > 0){ throw new ValidationException(invalidValues); } }

Yeah. So there is no planet in which the first branch would ever evaluate to true, and since debugging mode is nearly never enabled in production" well, at least they got to use reflection, I guess.

inedo50.png[Advertisement] Use NuGet or npm? Check out ProGet, the easy-to-use package repository that lets you host and manage your own personal or enterprise-wide NuGet feeds and npm repositories. It's got an impressively-featured free edition, too! TheDailyWtf?d=yIl2AUoC8zAdKurf0V-Bo4
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