A good friend asked me a quote today, and I thought I’d share it with you. Consider this code snippet:
01 02 03 04 05 06 07 08 09 10 11 12 | if (isThisAPremiumProductByIdInTheCatalogue(productId, products)) { // do something}..private static boolean isThisAPremiumProductByIdInTheCatalogue(String id, Map<String, String> products) { return products.get(id).equals(PREMIUM_PRODUCT);} |
The question I was asked was whether the above code was better for having the self-describing method, or whether the inline method refactoring should be used, as described on Refactoring Guru, and as originated by Martin Fowler. How interesting that the former website uses the same example as the latter… where the latter is from the original source.
Anyhow, this is an interesting dilemma. Is the following code better?
1 2 3 | if (products.get(id).equals(PREMIUM_PRODUCT)) { // do something} |
The second version is definitely more concise. The first version seemed to be word and operator soup…
However, if there were multiple places where this logic were needed, I’d want a method. I’d also want a method that didn’t talk so much. How about this?
1 2 3 | private static boolean isPremium(String id, Map<String, String> products) { return products.get(id).equals(PREMIUM_PRODUCT);} |
And you could leave it there…
Questions like this can end with something like what’s the form we find easiest to read right now? Generally, you want something that doesn’t take too much thinking about and which balances conciseness with explanation.
But What’s Going On?
The old tech lead trick is to answer the question at hand and then step back and say something like “What’s going on here, though?”.
In the above code, there’s this Map just lying around. In order to make sense of it we need a function to decide things. Is this a case of a missing type? Is there an anemic object model that’s too anemic?
Should we make a service to render out of the data some useful facts?
Anemic Model vs Data on the Loose
The days of smart objects are generally gone. We realise that if we put too much behaviour into model objects, they get too coupled with the wrong things. A model object should not, for instance, know how to save itself, because that just ends badly. Similarly, if you have a model object and some feature of the software has a preferred way of rendering it, but some other feature of the software has a different one – e.g. the difference between format on screen and format on disk, the model is probably not responsible for implementing the conversions to those formats.
The simplest solution to this is to have a very lightweight object model with getters and setters and other real basics, and then have services to transform that object model into other things, or pluck data from it.
I tend to consider things like JSON format to be native to an object model that was brought into life to represent the JSON data.
It’s generally obvious when an object is doing too much if it knows about more than one external technology that cares about representing it. That said, model objects that are annotated for two equivalent ways of rendering that exact model to JSON may be okayish…
And the whole point of the anemic object model discussion is to remind you that Map is not an object model. What we’ve got is data on the loose.
Data on the Loose
If you have some stuff in memory in basic collection classes and you need to create general purpose algorithms to mine that data, it looks like you may have a missing model object.
01 02 03 04 05 06 07 08 09 10 | public class ProductCatalogue { private static final String PREMIUM = "premium-o"; private Map<String, String> products; ... public boolean isPremium(String productId) { return PREMIUM.equals(products.get(productId)); } } |
One consideration, as hinted at above, is whether this designation of premium is an external opinion, or an internal facet of the model object. It looks like in this case, it’s an internal of how that model would explain its own inner representation.
In Summary
We should refactor mainly for clarity. This is why the refactoring book has opposites in it. For every inline function there’s an extract function you can also do.
However, always look at the natural separation of concerns and watch out for data on the loose!
Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Where to Put The Implementation Opinions expressed by Java Code Geeks contributors are their own. |
