I am reading the Monkhouse & Camerlengo book and I encountered this phrase:
Before building any class, it is worthwhile considering what it is that the class does. The same applies to methods - for each method, try to determine just what the method does. If you find yourself using the word "and" when describing a class or method, there is the possibility that the class or method is trying to be responsible for more than it should. You might then consider whether it makes sense to break a class or method into two or more classes or methods; it might make your code a bit more manageable and maintainable.
But for the Denny's DVD project, this method was used to retrieve all store's inventory:
I do not intend to criticize the authors, but this method clearly does more than one thing because it contains a boolean parameter.
So what is your opinion about this "do one thing" concept? Should we be so strict or should we allow some flexibility in designing classes/methods?
You should always try to adhere the 1 purpose for class (and method) principle. It will result in a clean design which can be easily tested, maintained, extended,... In short, all advantages of (good) object-oriented design. In practice that's not always possible. And it's a private method, so it's used for internal use only.
But a better design could be to create a similar 2nd method buildRecordNumbers which does exactly what the name suggest. The only drawback of this approach is that you'll have duplicate code (the for loop) with other business logic. That could be solved using a CallbackHandler for each method which performs the actual business logic.
Putting everything together could result in something like this:
But that's maybe a little bit of a design overkill, so for the sake of simplicity you could merge both methods into 1 using a boolean parameter. That brings us back to the original code
Thanks Roel, actually I have put your suggestion into the code for Denny's DVD project and it looks not only cool, but rather...an useful refactoring!
One of the disadvantages of the old design was that whenever you had to build the record numbers (calling getDvdList(true)), every time you ended up not using the return value (i.e. the List<DVD> dvdList). An useless collection of dvd's was filled up with each method call and the garbage collector had to clean this list every time with no sense...
Furthermore, boolean arguments look bad for the reader. Every time when you see a method call with the boolean argument you must look deeper into the code to see what is the use of that boolean argument.
With your proposed design things look more straightforward.
Alex Dragoi, SCJP 1.4, OCPJP 6
Those who dance are thought mad by those who hear not the music. This tiny ad plays the bagpipes:
Devious Experiments for a Truly Passive Greenhouse!