3 ms·

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 p

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.

whstl 2y ago
To quote from Clean Code: "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"); } }
- nyrikki 2y ago
  Thank you for doing the work to find that for me. I still don't see that as: > "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.
  - whstl 2y ago
    > 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.
    - nyrikki 2y ago
      In that example the constructor parameters were just moved, is there some cost in overloading the empty params? Original (labeled bad by the book): private void printGuessStatistics(char candidate, int count) { String number; String verb; String pluralModifier; Modified version: public class GuessStatisticsMessage { private String number; private String verb; private String pluralModifier; public String make(char candidate, int count) { createPluralDependentMessageParts(count); return String.format( "There %s %s %s%s", verb, number, candidate, pluralModifier ); } Once again, it was presented in the context of naming things....not object or data structures. Often one has to resort to spherical cows to explain things. (Edited to add the overloaded constructor params)
      - whstl 2y ago
        > 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. [1] Classic parody/criticism of the style here: http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom...
nyrikki 2y ago
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.

whstl 2y ago

To quote from Clean Code: "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"); } }

nyrikki 2y ago
Thank you for doing the work to find that for me. I still don't see that as: > "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.
- whstl 2y ago
  > 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.
  - nyrikki 2y ago
    In that example the constructor parameters were just moved, is there some cost in overloading the empty params? Original (labeled bad by the book): private void printGuessStatistics(char candidate, int count) { String number; String verb; String pluralModifier; Modified version: public class GuessStatisticsMessage { private String number; private String verb; private String pluralModifier; public String make(char candidate, int count) { createPluralDependentMessageParts(count); return String.format( "There %s %s %s%s", verb, number, candidate, pluralModifier ); } Once again, it was presented in the context of naming things....not object or data structures. Often one has to resort to spherical cows to explain things. (Edited to add the overloaded constructor params)
    - whstl 2y ago
      > 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. [1] Classic parody/criticism of the style here: http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom-of-nouns.html http://steve-yegge.blogspot.com/2006/03/execution-in-kingdom...

nyrikki 2y ago

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.