Reckless Refactoring.
I see a lot of tests where you call A(), which does nothing but call B(A.somemember), which merely calls C(Amember, C.member1, B.Member1, B.Member2, C.Member2) which does nothing but call D(Amember, Cmember1, Bmember1, Bmember2, Cmember2, Dmember). That's not a problem, that's a symptom. How are we to think about such things?
That is removal of duplication and I suppose that in some sense it is "simplifying" the calling of D() for the client who uses A(). In that regard I suppose it's trying to keep the test code clean. But I'm never sure that it's making things simpler. I fear that tests tend to gather reckless refactoring, making it easier to write the one test you're working on without real concern for overall design simplicity. I worry about reckless refactoring. It adds to the cognitive load of your system in several ways, and requires quite a bit of unraveling or navigation to make sense of it.
Also to the defense of long chains, I have seen similar patterns in the production code (adding in a few abstract classes, necessary to mocking). I guess that means that it's treated just like production code. Of course, the navigational burden for understanding the production code can be far worse. The difficulty is partly due to the decoupling of classes for the purpose of testing (esp. with mocks).
I worry about us sometimes. Are we making localized optimizations for test-writing at the expense of increased congnitive load? What happens when we see new people come to the TDD projects late? How will this stand up as a legacy?
Of course I worry a lot. It's my way. I have high expectations, and my analysis of current situations seldom compares favorably to the expectations I have. But I think there's a problem in here that has a better solution than "stay out of those places."
Our refactoring is supposed to be relentless, and always toward building a more simple model for further design evolution. If we refactor to make things easier, instead of making things simpler, are we committing the greater design sin?
!commentForm
You could do this:
a.getB().getC().getD().somemethod(a.getB().getC().somemember, a.getB().somemember)
I guess that's better.
a.getB().getC().getD().somemethod(a.getB().getC().somemember, a.getB().somemember)
I guess that's better.
David, I think your example: a.getB().getC().getD().somemethod(a.getB().getC().somemember, a.getB().somemember)
is flawed since it violates the Law of Demeter. It ends up coupling the client of A with its details. Exactly what you do not want to do. The example Tim provides, if I read it correctly, has A delegating to B, B delegating to C and so on. This hides the details. Doesn't that generally lead to a cleaner solution?
is flawed since it violates the Law of Demeter. It ends up coupling the client of A with its details. Exactly what you do not want to do. The example Tim provides, if I read it correctly, has A delegating to B, B delegating to C and so on. This hides the details. Doesn't that generally lead to a cleaner solution?
David is joking with you. But thanks for the comment.
Hiding is only so good. You can hide things too deep, and through too much abstraction to make a test meaningful. I would like something along the lines that Paul P. recommends, where the test actually tells you something, not the kind where you have to go deep-code-spelunking to figure out what question the tests are asking.
Hiding is only so good. You can hide things too deep, and through too much abstraction to make a test meaningful. I would like something along the lines that Paul P. recommends, where the test actually tells you something, not the kind where you have to go deep-code-spelunking to figure out what question the tests are asking.
The original statement may have hidden details in the first call, but they're glaringly obvious in the later calls: it's still a Law of Demeter violation regardless of how you factor it.
Something like this seems to call for sitting back and actually thinking about the design. It doesn't seem to matter how you package it; something knows something else's internals, and that's a design problem.
John Roth
Something like this seems to call for sitting back and actually thinking about the design. It doesn't seem to matter how you package it; something knows something else's internals, and that's a design problem.
John Roth
I was actually more concerned that we have a chain of five of six functions, each of which has only one use, and each of which does nothing but call the next with one or two additional parameters. The issue isn't that it's not going to work, or that it's not encapsulated, but rather than it's deep and has to be navigated pretty deeply to know what is going on. Actually, it's worse because it's not always that straightforward, since some of the calls are through interfaces. It took longer to navigate to the actual code than to figure out what the actual code was doing once you got there -- and that's from a routine that does nothing but call the actual code way down the line.
Looks like an abstract base class should be in there somewhere, or a single class that has many overloads for one method.
Add Child Page to RecklessRefactoring