Or maybe there is something that we can do about it? Something that will open our code for extensions?
Today I want to show you how you can achieve that.
But first, let me explain why this instanceof is a problem at all.
Take a look at the code
Today we will talk a bit about this code:
public class ChangeProcessingHandler {
public void triggerProcessingChangeOf(Code code, Change change) {
verifyChangeOf(code, change);
if (change instanceof Refactoring) {
processRefactoring(code, (Refactoring) change);
} else if (change instanceof Improvement) {
processImprovement(code, (Improvement) change);
} else if (change instanceof Growth) {
processGrowth(code, (Growth) change);
} else {
throw new UnsuportedChangeException();
}
}
// some more code
}
And we will try to improve it.I tried to make this code descriptive, but let me just briefly summarize it.
Depending on a specific type of the Change interface’s implementation, we choose an accurate way of processing. In case of not finding the type that matches, we just throw an exception.
Now, let’s take a look at what are the problems with this code.
Interface and its implementations?
When you look at the method’s declaration, what can you say about it? That it needs two input parameters, that’s for sure. What kind of information does it give us? We know dependencies and based on their API we know how in the body of the method we may interact with those passed objects.Is it true in the given example? Unfortunately not. We are passing an instance of Change and we expect that the body of the method will depend on its interface. But inside we are casting our instance into a specific type, which results in an increased number of dependencies.
This itself is not a good design decision, but what is even worse - we increase this number behind the scene. Until you won’t read the body of the method you won’t know that.
This lack of knowledge is far more worse than the number of dependencies.
New type is not so easy to add
Let’s imagine that you have to add a new implementation of the Change interface. What will happen? Well, nothing. You will add the class definition and tests for it. You will run all tests. You will be lucky if there is at least one component or system test that will reach the presented code with the newly introduced implementation of the Change interface and will fail.The problem starts when there is no such test and you won’t even know there’s a place you should change in order to meet new functionality.
Everything will compile and you will just work till…
Exception? Why?
You notice this nice UnsupportedChangeException in the code? To be honest, it’s there only because of a wrong design.There is only one reason why we have it. Exception prevents us from adding a new type and forgetting to add support of newly introduced functionality in there. Assuming there is at least one test that will fail in such situation.
Why have I called it wrong design? Well, using an exception to signalize the need of support of new functionality is rather misusing of exceptions. I also believe that would be far more better if our code would signalize such thing by not compiling. It would make sense for me and definitely would give faster feedback.
Visitor for the rescue!
Visitor allows us to add an additional functionality whose implementation depends on specific type of the object. It allows for that with the usage of an interface’s method. Thanks to that we can avoid retrieving information about specific interface’s implementation on our own.First off, we need to make it possible to retrieve information about the type of an object. To do so, we have to add to our interface one method which will allow us to pass a visitor:
public interface Change {
void accept(Visitator visitator);
}
It is implementation in each object that implements an interface is pretty straightforward:
public class Refactoring implements Change {
@Override
public void accept(Visitator visitator) {
visitator.visit(this);
}
// some code
}
What we may observe by looking at the line where we have got invocation of a method visit()? This is the place where information about type is retrieved. There’s no need for instanceof, no need for casting. This is what we get for free with the support of better design.At this moment, you probably know how interface of Visitor looks like:
public interface Visitator {
void visit(Refactoring refactoring);
void visit(Improvement improvement);
void visit(Growth growth);
}
Not so complicated, isn’t it?After this we have to extract some code from ChangeProcessingHandler class to the class that implements our Visitor interface:
public class ChangeProcessor implements Visitator {
private final Code code;
public ChangeProcessor(Code code) {
this.code = code;
}
@Override
public void visit(Refactoring refactoring) {
// some code
}
@Override
public void visit(Improvement improvement) {
// some code
}
@Override
public void visit(Growth growth) {
// some code
}
}
And of course we have to use this in the right place:
public class ChangeProcessingHandlerRefactored {
public void triggerProcessingChangeOf(Code code, Change change) {
verifyChangeOf(code, change);
change.accept(new ChangeProcessor(code));
}
}
Is it better?
Ok, so we changed our original code. Now let me explain what we’ve gained.- We’ve just got rid of an exception. It is no longer needed because required support for newly introduced implementation would be signalled by non-compiling code.
- Fast feedback is a result of using interfaces that will tell us what more we have to implement to have everything fully supported.
- Single Responsibility Principle comes into play because each specific implementation of Visitor interface is responsible only for one functionality.
- Design is behavior-oriented (interfaces), not implementation-oriented (instanceof + casting). In this way we are hiding implementation details.
- Design is open for extensions. It is really easy to introduce new functionality which implementation differ for specific objects.
It is not so perfect
Each design is a tradeoff. You get something, but it comes at a cost.I listed benefits in the previous paragraph, so what about the cost?
- So many objects
One may say that is an obvious result of using any design pattern and I would say yes. However it does not change the fact that with increased amount of objects it is harder to navigate through them.
Having everything in one object may be a problem, but not well named or disorganized classes may result with a mess.
-
Complexity
All of those objects need a name and it’s great if these objects are domain related. In such case we end up with a better understanding of our application. But it is not always the case.
Also we have to be very careful with naming the newly introduced classes. All of them have to be named in a self-explanatory manner. Which is not as easy as some may think.
-
Where’s my (bounded) context?
Visitor may help with problems similar to the one presented in the example. But if there’s a lot of places like that you have to realize that each visitor is in some way of putting a behavior of the object into another object. What about Law of Demeter? What about Tell, don’t ask?
Before you will use visitor to solve instanceof problem you should ask yourself is this functionality not a part of the object itself? Some developers explains me that’s a way of having small objects. Well, for me such explanation is a proof that we should think about Bounded Contexts instead. Objects still would be small and their behavior would not leak to the outer class.
That’s all, folks
That’s all for today. I hope that you found this idea of redesign useful and that after you’ve read this article, the smells in your code will definitely feel endangered.As always, I encourage you to write comments and share your perspective and experiences. Maybe you know more about benefits/problems related to such change.
No comments:
Post a Comment