4 ms·

> I’ll be discussing the very first example of refactoring in his book from “Chapter 2: Meaningful Names”. I prefer the 'after' (not that I don't have complain

by mrkeen 2y ago

> I’ll be discussing the very first example of refactoring in his book from “Chapter 2: Meaningful Names”. I prefer the 'after' (not that I don't have complaints about it). When I read 'before', I can't see any structure. I scan from top-to-bottom, get half-way down, think "wait, what?" and my eyes jump back up without me even consciously deciding to. > The first thing you’ll notice is that Martin has taken a single, mostly PURE function (shout out to all the functional bros), and made a class out of it. That doesn't make a good purity argument. 'Before' has a print. There is no way to test it apart from reading stdout (or god forbid, having your test harness somehow intercept/mock the print call). `After` has no such IO, it returns the String itself to make unit testing possible. I despise mutability, but neither before nor after does any re-assignment, (only assignment.) So really, those fields should be marked 'final' and the 'private void' methods should be constructors or something... I'm not really sure about the 2008 version of whatever language he's using.

Joker_vD 2y ago
No, that refactored version is indeed supposed to be used like print(new GuessStatisticsMessage().make(candidate, count)); There are cases where using a class as a bag of pseudo-global variables that get mutated during a long web of internal method calls — a recursive-descent parser for a non-trival language is a prime example, — is fine but this, I'd argue, is not one of those cases. You need to (very straightforwardly) map two input values to three output values and return them glued together — one function with a switch in it is quite enough for that. P.S. And what the hell does "thereAreNoLetters" even mean? Oh, right, it makes sense since this is a letter-guessing game, and this class is used to tell you that no, there are no letters like the one you tried to guess. Bleh.
viraptor 2y ago
> There is no way to test it apart from reading stdout (or god forbid, having your test harness somehow intercept/mock the print call). It's trivial to change if you care about that part. DI - pass in a ConsoleWriter. It also solves the test without mocking.

by mrkeen 2y ago

Joker_vD 2y ago

No, that refactored version is indeed supposed to be used like print(new GuessStatisticsMessage().make(candidate, count)); There are cases where using a class as a bag of pseudo-global variables that get mutated during a long web of internal method calls — a recursive-descent parser for a non-trival language is a prime example, — is fine but this, I'd argue, is not one of those cases. You need to (very straightforwardly) map two input values to three output values and return them glued together — one function with a switch in it is quite enough for that. P.S. And what the hell does "thereAreNoLetters" even mean? Oh, right, it makes sense since this is a letter-guessing game, and this class is used to tell you that no, there are no letters like the one you tried to guess. Bleh.

viraptor 2y ago

> There is no way to test it apart from reading stdout (or god forbid, having your test harness somehow intercept/mock the print call). It's trivial to change if you care about that part. DI - pass in a ConsoleWriter. It also solves the test without mocking.