> make a lot of small tiny methods which are one liners or two liners.
I'm presuming you mean public tiny methods? Having private ones like that can be good if makes sense to do so (encapsulates logic, increases readability, etc)
I never believed this, even when I was compelled to do it.
What are we achieving in these plethora of small methods? There are many potential negative patterns that eventuate.
- Lifting variables into “global” (object) state. In more complex classes, it’s often hard to even identify that this has happened. Is this permanent object state, or just an internal temporary variable that made it easier to leap around methods?
- Making code harder to change as variables must be lifted into method parameters (so changing a variable, or adding a new one leads to multiple modifications). Method1 calls Method2 calls Method3 with a “dependency” that Method2 never needs.
- Poor naming leading to obtuse code. DoThingPart1, DoThingPart2
- Harder to follow code by having to jump around the file (or worse, multiple files).
There are better and worse ways to structure code to make it easier to read and reason about, however blind metric approaches are not the way.
I don't remember anything about lifting state up in the clean series, was it perhaps a react specific book?
To me that would violate the dependency inversion principle that most of the books leverage heavily.
I know that some languages like .net encourage those Singleton classes, but I would appreciate being pointed at where the clean series sells this.
I am of the bounded context camp for component sizing in general so it is likely I skimmed and dumped the concept, like I did with the over reliance on polymorphisms, which is a sometimes food in my mind.
"The function is a bit too long and the variables are used throughout. To split the function into smaller pieces we need to create a GuessStatisticsMessage class and make the three variables fields of this class." - Add Meaningful Context, Page 28
EDIT: And then right below there's an example where the author lifts variables into object state.
EDIT 2: Funny enough, ChatGPT managed to refactor to something even much shorter and IMO much clearer than both examples in the book:
private void printGuessStatistics(char candidate, int count) {
if (count == 0) {
print("There are no " + candidate + "s");
} else if (count == 1) {
print("There is 1 " + candidate);
} else {
print("There are " + count + " " + candidate + "s");
}
}
> "Lifting variables into “global” (object) state"
It is simply the inversion and extraction method that is commonly used. The value of it is lost here as his example is poor IMHO as I find that cleaning up deep nested arrow code is where it is.
This method on page 28 is about refactoring to improve readability, and the location where the variables are declared are still in the same class.
Nothing has changed in that example except adding named private methods in place of logic inside an if-elif-else ladder.
So this:
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
}
else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
}
else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
Is changed to this:
if (count == 0) {
thereAreNoLetters();
}
else if (count == 1) {
thereIsOneLetter();
}
else {
thereAreManyLetters(count);
}
Remember this chapter is about "name things" not flow control or even data flow.
It actually is intended to help with most of the concerns above and has nothing to do with the react style anti-parameter drilling style of 'lifting'
If you go to page 97 he goes over the 'The Law of Demeter' and argues against exactly what was above and actually cites Martin Fowlers refactoring book which is written in a far better style and tries to call out nuance.
So my opinion that he gives semi-helpful advice that he over sells as received wisdom still holds. Obviously the cost of calling a private method in your language and how that impacts your use case matter here.
> This method on page 28 is about refactoring to improve readability, and the location where the variables are declared are still in the same class.
Same class, but method-scope local variables were unnecessary turned into long-lived instance variables. This is what GP means by "Lifting variables into “global” (object) state". Their phrasing, not mine.
This unnecessary usage of state for what could be a simple variable binding is what GP is criticizing, and they're arguing that is problematic for maintenance and readability. I agree with them.
> In that example the constructor parameters were just moved
Not really. In the "Original" example there's no constructor in sight, as that's just a standalone procedural-style method, not a class.
The second one turns the method into a class.
> is there some cost
As GP mentions, this incurs mental costs: "In more complex classes, it’s often hard to even identify that this has happened. Is this permanent object state, or just an internal temporary variable that made it easier to leap around methods?"
In terms of performance, this depends on the language/runtime/type, but in general you'll get heap allocation instead of a stack, and it will use the GC.
Also if the lifetime of the private fields is the same of GuessStatisticsMessage, so you'll waste memory if they're complex objects that don't need to live for as long as the class. Depends, YMMV. I once had a memory leak-ish issue due to that in a Ruby app.
EDIT:
> Once again, it was presented in the context of naming things....not object or data structures
This is fine, but the example is not even good in terms of names. `make` is a terrible method name [1], and, turning local variables into long-lived object variables doesn't improve their names, it only muddies the waters like GP mentioned.
It all doesn't matter, these are code fragments that are not in context of a full program.
Lets look at the labels on these code blocks (in a chapter called "Chapter 2: Meaningful Names")
> Listing 2-1
> Variables with unclear context.
> Listing 2-2
> Variables have a context.
What part of that would even suggest that they are indicating the most performant, idealized code?
They are non-complete random code fragments meant to discuss one of the hardest problems in computer science...naming things.
It is simply a poor example of "Variables have a context" being more readable than "Variables with unclear context", not a suggestion that the lifting principle is required.
This is _simply_ an example that is similar to the extract and inversion method that all of us that get brought in to break up legacy monoliths have to do because often things are a big ball of much with vars that look more like it was written in the language Brainfuck or obfuscated Perl than anything reasonable.
That is not anything that the Clean camp, or even Bob came up with...it is often the reality when trying to save systems from architectural erosion or....
But seriously if you consider 'clean code' as cargo culting design choices without though from any source, you are going to be screwed no matter what.
The number of competing needs related to a dozen types of cohesion along with a dozen types of coupling will never reduce to a simple set of rules.
Strings are interned either in the string pool, or even if you use New...which this code wasn't, anything newer than 8u20 will dedupe and duplicate strings will only have a single instance even if they aren't in the string pool. So long lived strings total space usage isn't really massive.
If you choose to save the size of a pointer with what appears a short lived single computational class anyway, sacrificing readability and maintainability, you will have far greater problems than inefficient memory use.
The parameters have moved to a class method, still within the class, which is now public, but still taking a string and an intrinsic int, the string, as being passed as a parameter is already in the string pool or perhaps on the heap outside the string pool if it was created with New,
Nothing has been elevated outside of the block scope.
Where is anything that was claimed about raising variables to global scope as was suggested above apply.
You still have a constructor btw, just the make() method overloading the missing class construction.
The parameters are just placeholders, they have to be instantiate when invoked even if the implicit construction is hidden.
But in the beginning example and the refactored example they have the same instance variables, with the latter being marked private but that doesn't impact the lifecycle.
The question being if the refactored version uses the new private methods as callable or if optimization removes them.
The point being is that blaming performance regressions on this type of refactoring to improve readability is on thin ground for java.
This example does nothing to suggest that global state APIs etc.. that are a problem in react relate at all to the recommendation, which are still containing everything in a local scope.
I thank you for taking the time to answer, but what I am looking for is how this somehow lifts vars to a global scope, which it doesn't.
Ya looking into this more this is a react hyper correction to the Law of Demeter, related to language limits vs broad concepts.
Not being a UI guy, I am not the one to say how universal this is, but the react context API seems to use stamp coupling and has to propagate those changes globally and is expensive.
Good example of where context is King with tradeoffs.
For example in Ruby land it is very common to make a class and then make a lot of small tiny methods which are one liners or two liners.
I had asked him directly about this and his answer was to avoid doing it.
Since then my Ruby and Common Lisp code has become much better.
I have since moved to rust, but the point still applies.