- Image by B Tal via Flickr
How confused can a discussion get? As confused as the discussion in the comments to Benjamin Eberlei’s Explicit Code requires no comments – Only bad code does. This discussion has a fake identity, and nobody seems to notice. As you can see, the blog post claims to be about code comments, but it isn’t. The example given does not adress the issue of commenting. In the refactored version, Benjamin has not removed the comment from the original, and done nothing (OK, a tiny bit) to replace it with expressive code. Since the example is irrelevant to the subject matter, it fails to keep the discussion grounded, allowing it to degenerate into dogmatic opinion and subjective speculation.
Also contributing to the confusion is an apparent lack of understanding of the process of refactoring. Refactoring is not a deterministic process, nor is it a strait jacket. You have choice, and you should learn to exercise it judiciously. You take a chunk of code, change it somewhat, and then you decide whether there’s actually been an improvement. If there hasn’t, as many think in this case (and I’m somewhat inclined to agree with them, although Benjamin has great principles and valid points), you have three choices:
- Refactor further, hoping to arrive at something more satisfactory.
- Undo the change and try something else.
- Undo the change and keep the original version if you think that’s the best you can do.
These are the normal choices for those of us who refactor routinely. It happens often enough. Even if you undo the change, it doesn’t necessarily mean you’ve wasted your time. You’ve probably learned something.
In this case, too, there is a chance to learn something worthwhile if you can distance yourself from the noisy confrontation. The most obvious thing to observe is that trying to extract methods from a 6-7 line method is a suicide mission in the circumstances. It’s bound to increase the volume of code a lot, and it’s only natural that the result gets labeled “bloated”. If you do something similar with a longer method, the percentage increase is much less, and you have a chance of not drowning in irrelevant criticism.
Still… You can’t refactor without unittests in place. Everyone always forgets about the unittests when talking about refactoring. And that takes me to my point, which also has nothing to do with the original argument about code needing or not needing any kind of comments.
And I guess you’re right.
If that original snippet was designed with testing in mind, it never were one function in the first place, let alone a constructor. It wouldn’t be a bunch of method on one class either, because a lot of separate responsibilities are intertwined in it: XSL transformation (possibly in the parent class, the file resource lookup, and the interpretation of the pathinfo). So I guess it’s really some sort of a “strawman snippet”.
And then again, anyone can argue that it’s really not worth it, to blow up that snippet to THREE classes (and possibly at least two interfaces, to loosen the coupling between said classes). But eventually you’ll get to a state, where that code is not only readable, but easily testable. Because that code, in its original form is not easily testable. It does work in a constructor, talks to global variables (via a singleton), and to the filesystem. (I recommend http://misko.hevery.com/ to everyone, lots of great posts about testable design.)
upsz, I screwed up the parentheses in the middle. it should read: …XSL transformation (possibly in the parent class), the file resource lookup, and the interpretation of the pathinfo…
You’re absolutely right, I agree with all of what you’re saying. You touch on several things I’ve been thinking about. Above all, the discussion of comments is going on without a foundation. Unless you understand code smells and refactoring properly, the idea of using fewer comments is probably more or less incomprehensible.
In a comment to Benjamin Eberlei’s post I suggested something similar to what you’re saying: Instead of his refactoring, you could extract a class to handle the file system aspect. Another way of looking at it is that the original code does not suffer from the Long Method smell. Primitive Obsession is more like it. Therefore Extract Class should be more relevant than Extract Method.
Has it occurred to you that perhaps Marco Tabini and Associates does in fact unit test? Especially since he has blogged about this explicitly?
There is nothing about the design of his simple, 5-line constructor that is difficult to unit test. The whole “debate” over the idea of commenting as good or bad is laughable.
Intelligent developers write good code and good comments, and all the dogma is irrelevant to making this possible. Experience and careful study of the mistakes of the past (which includes design patterns and all the other solutions to learning from mistakes) are all that is needed, end of story.
@Greg: No one except you has generalized about who writes unit tests and who doesn’t. This is a discussion about a specific code example, not an attempt to insult the people who developed it. It’s just a piece of code; I don’t care who wrote it, and have nothing against Marco Tabini and associates. It might as well have been my own. My code is not always 100% perfect (what a surprise!). In short, there is absolutely no reason to take this personally (on behalf of someone else).
The tone of your comment makes me wonder what’s behind it. And if you think the discussion is a waste of time, why participate?
I was a tad bit nearer to insulting Marco (not a tiny bit intentionally), so allow me to explain myself:
I have read Marco’s post about testing, so yes, it has occured to me that he does indeed unittests his code
Actually testability can be measured, and a constructor that does those things I’ve enlisted, is more difficult to test than a constructor that does not do these things.
Constructor does real work: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/ is a very good reading, skim through it
Your other points are infalsifiably truthful, though
[...] Allen’s opinions on Benjamin Eberlei’s [...]
[...] week, “On Code Commenting and Technical Debt” raised a lot of response throughout the community. I think that discussion is great, and I’m all for a debate that enhances the community. But [...]