Forums
Today's Java snippet comes from Capybara James.
The first sign something was wrong was this:
private Map<String, String> getExtractedDataMap(PayloadDto payload) {
return setExtractedDataToMap(payload);
}
Java conventions tell us that a get method retrieves a value, and a set method mutates the value. So a getter that calls a setter is… confusing. But neither of these are truly getters nor setters.
setExtractedDataToMap converts the PayloadDto to a Map<String, String>. getExtractedMap just calls that, which is just one extra layer of indirection that nobody needed, but whatever. At its core, this is just two badly named methods where there should be one.
But that distracts from the true WTF in here. Why on Earth are we converting an actual Java object to a Map<String,String>? That is a definite code smell, a sign that someone isn't entirely comfortable with object-oriented programming. You can't even say, "Well, maybe for serialization to JSON or something?" because Java has serializers that just do this transparently. And that's just the purpose of a DTO in the first place- to be a bucket that holds data for easy serialization.
We're left wondering what the point of all of this code is, and we're not alone. James writes:
I found this gem of a code snippet while trying to understand a workflow for data flow documentation purpose. I was not quite sure what the original developer was trying to achieve and at this point I just gave up