Java Community Antipatterns, an Ongoing Series
Published at 17:27 on 1 September 2023
To give you an idea of the general pathetic hilarity of the situation, I was reviewing some code at work today. It reads in a message from Kafka, obtains a validator object, and calls that object’s isValid() method on the message it receives. That method in return a ValidationResult object, whose valid() method is then called inside an if statement.
This immediately strikes me as odd. When you validate something, it either turns out to be valid or invalid. That’s it. Two options, no more, no less. Yes/no. Black/white. On/off. There is no need to create a new data type to represent a validation result, because a perfectly appropriate data type already exists, built in to Java: the Boolean. Just use that. Far simpler and cleaner.
Maybe the ValidationResult object does something special and has extended features beyond those of a Boolean? Yes, it has a message field! But wait, that field is never accessed. The only thing that is ever done with that object is to call the valid() method, whose purpose is to return the Boolean value that should have been used in the first place.
And what of the validator object? Its class definition is very simple, just one short method that makes some basic checks. If its argument turns out to be invalid, the message part of the result is set to the string “Data is not valid.” No, I am not making this up. Of course the data is not valid, you moron! That is why the valid flag is set to false! This field conveys exactly zero meaningful information.
What other code uses this validating logic? None of it, it turns out! So there was no need for the validator class, either. Could have just added a private isValid() method inside the one (short) source file where this logic is used. Would have been a whole lot clearer, because the person reading the code wouldn’t have had to open another file to determine just what the validation logic is.
So three classes, and three source files, are being used where just one would have sufficed.
Now, this was a particularly egregious example, but this sort of crap-ola happens over and over (and over) again in Java code. Needless complexity everywhere.