- cross-posted to:
- [email protected]
- cross-posted to:
- [email protected]
Seeing that Uncle Bob is making a new version of Clean Code I decided to try and find this article about the original.
Seeing that Uncle Bob is making a new version of Clean Code I decided to try and find this article about the original.
Why is it a
voidmethod? This only tells me that some state is mutated somewhere, but the effect is neither visible nor documented.I would expect a function called “calculate” to just return a number and not have any side effects.
You’re nitpicking.
As it happens, it’s just an example to illustrate specifically the “extract to method” issues the author had.
Of course, in a real world scenario we want to limit mutating state, so it’s likely this method would return a
Commissionlist, which would then be used by a Use Case class which persists it.I’m fairly sure the advice about limiting mutating state is also in the book, though.
At the same time, you’re likely going to have a void somewhere, because some use cases are only about mutatimg something (e.g. changing something in the database).
It’s not nitpicking, stuff like this is far more impactful than choosing between 5 lines vs 10 lines long methods, or whether the
hasExtraCommissions“if” belongs inside or outside ofcalculateExtraCommissions. This kind of thing should immediately jump out at you as a red flag when you’re reading code, it’s not something to handwave away as a detail.I never claimed it’s not important, I’m just saying it’s not relevant here, as there is no context to where this method was put in the code.
As I said, it might be top-level. You have to mutate state somewhere, because that’s what applications ultimately do. You just don’t want state mutations everywhere, because that makes bad code.
The whole book is like this, though, and these are specifically supposed to be examples of “good” code. The rewritten time class toward the end, a fully rewritten Java module, is a nightmare by the time Martin finishes with it. And I’m pretty sure it has a bug, though I couldn’t be bothered to type the whole thing into an editor to test it myself.