The Chameleon Function
In Doctor Who the character Kamelion was a robot which could take any form. The physical prop they used was allegedly very troublesome and only understood by its creator, who wasn’t around to help put it right.
So to the Chamelion function …
Consider this code:
01 02 03 04 05 06 07 08 09 10 11 12 13 14 15 16 17 18 | public List<Document> getAllDocuments(Request request, int userId, String field) { Query q = createQueryFrom(request); switch (field) { case "title" : q.addCriteria(Criteria.where( "title" ).is(request.getTitle()); break ; case "name" : q.addCriteria(Criteria.where( "name" ).is(request.getName()); break ; default : throw new IllegalArgumentException( "Bad field: " + field); } return q; } |
There’s a fair bit going on above. Let’s just understand it. Some sort of request comes in, and we can make a basic query from it. Then based on the field provided by the caller, we add a criterion to the query using that field and pulling an operand out of the request.
On top of that we have to throw an error if the caller provides a field we don’t know how to query.
What’s wrong with this function?
I’ll Tell You What’s Wrong…
It’s not a function. It’s two functions. See also Both Kinds of Music.
The calling code might look like this:
1 2 3 4 5 | // one call site getAllDocuments(request, id, "title" ); // another getAllDocumetns(request, id, "name" ); |
We’re using a choice of string to control half the flow of a single function.
It’s worse than that… we need an exception to throw when some caller invents a string we’ve never heard of.
Let’s just refactor this a second:
01 02 03 04 05 06 07 08 09 10 11 | public List<Document> getAllDocumentsByTitle(Request request, int userId) { Query q = createQueryFrom(request); q.addCriteria(Criteria.where( "title" ).is(request.getTitle()); return q; } public List<Document> getAllDocumentsByName(Request request, int userId) { Query q = createQueryFrom(request); q.addCriteria(Criteria.where( "name" ).is(request.getName()); return q; } |
By splitting this into two functions, it’s self-documenting, easier to follow and doesn’t need to handle rogue strings. It’s probably slightly faster, but that’s not really a major driver.
But What About The Duplication?
I suspect that one driver to chameleon functions is a misguided attempt to reduce code duplication. Please note that the above has examples of code being reused across the two functions – createQueryFrom but has independent logic in each. It’s not duplicated code.
The example I drew this from may originally have had more than one line of code where we now see createQueryFrom this may have driven a feeling of fear of duplication, which in turn created the monster. Refactor relentlessly to reduce the right duplication and these sorts of things won’t happen.
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: The Chameleon Function Opinions expressed by Java Code Geeks contributors are their own. |