5 ms·

This is why I hate plug-ins that claim to produce readability or code-cleanliness metrics, because they very typically have the opposite effect. A 1000 line fu

This is why I hate plug-ins that claim to produce readability or code-cleanliness metrics, because they very typically have the opposite effect. A 1000 line function broken down into 100 different 10 line functions which are then each called in turn (and need lots of parameters to pass around the inevitable shared state) is actually often less readable than just a 1000 line function. The way to deal with such a function to ask why there needed to be a 1000 line function. Why does so much apparently need to be done here and now and so much state shared across so many lines. Code metrics tools would rather you break everything down into micro-functions which if you're blindly trying to follow the tooling recommendations to get it past a check-in gate will probably end up badly named and with little thinking into actual abstraction. If you have two functions but whenever you call function foo = Bar() you have to then call function Baz(foo) then you don't really have two functions at all.

SemanticStrengh 4y ago
This. A related term being ravioli code.
danuker 4y ago
For what it's worth, my take on it is that you should extract whatever bits of pure functions you can find in the 1000-line ones. This would help testing and understanding. You can test and understand the parts independently, and only have to test the huge function in order to check that the parts are attached correctly. https://rhodesmill.org/brandon/talks/#hoist https://rhodesmill.org/brandon/talks/#hoist https://danuker.go.ro/the-grand-unified-theory-of-software-architecture.html https://danuker.go.ro/the-grand-unified-theory-of-software-a...
- andi999 4y ago
  Only if the pure function have a few parameters and do not need to carry the world.
  - danuker 4y ago
    You can bundle the parameters into plain/pure/humble objects where it makes sense. https://khanlou.com/2014/12/pure-objects/ https://khanlou.com/2014/12/pure-objects/ http://xunitpatterns.com/Humble%20Object.html http://xunitpatterns.com/Humble%20Object.html
    - mannykannot 4y ago
      'Where it makes sense' is key - otherwise you are just stuffing the wiring behind the desk, adding to the difficulty of understanding it.
- marginalia_nu 4y ago
  This a bit of an aside, but I wonder if people would be less prone to advocate pure functions if they had a name that didn't sound like a paladin had come up with them. Would people as eagerly advocate rewriting code to break it into greasy functions?
  - danuker 4y ago
    "Side-effect-free" would be a mouthful, and I don't know any other name that is both accurate and well-known.
    - marginalia_nu 4y ago
      Well pure is just as arbitrary as greasy, isn't it? Except one has positive connotations and the other has negative ones.
- Verdex 4y ago
  Well not necessarily just 'pure'. You also want unsurprising. For example imagine a pure function that takes a string and returns a string. The returned string has every character lower cased. Okay name it to_lower and move it out. But now imagine that it returns the complete text of Shakespeare if the input it 'bob' (why? customer requirement. their business literally falls apart if this doesn't happen). In this case you probably shouldn't pull it out into it's own thing because it's so specific (and weird) that you'll want it nearby it's use case (and not available for developers to accidentally use for the non Shakespeare uses). * yeah this is convoluted. There are better ways to factor such a function. However the point is that something can be 'pure' but still not a good candidate to be pulled out of context.
  - dolni 4y ago
    > In this case you probably shouldn't pull it out into it's own thing because it's so specific (and weird) that you'll want it nearby it's use case (and not available for developers to accidentally use for the non Shakespeare uses). I guess it depends on how much this weird Shakespearean to_lower is used around the codebase. If there is one spot where it's used, I would probably factor out to_lower and add a special case check for "bob" in the business logic code (outside the scope of to_lower). If the Shakespearean to_lower gets used in various places it actually might make sense to be factored into its own function.
    - Verdex 4y ago
      * yeah this is convoluted. There are better ways to factor such a function. However the point is that something can be 'pure' but still not a good candidate to be pulled out of context.
- usrbinbash 4y ago
  > For what it's worth, my take on it is that you should extract whatever bits of pure functions you can find in the 1000-line ones. Do these bits of pure functions appear anywhere else in the code? If so, sure, I happily pull them out into their own functions. If not, I will leave them exactly where they are. It's easier to read a laundry list than to look up many different parts in isolation without context. As for testability: If some bit of code only appears in that one long laundry-list function, what do I gain by testing it outside of that context?
  - Dudeman112 4y ago
    That's acceptable if you have really good reasons to assume the requirements won't change next week, which happens almost never. 1000x1 lines of code all at once have a lot of space for hidden non-obvious interactions. Maybe it won't have that, but I haven't ever encountered it. Making it 10x100 lines of code won't necessarily make it better, but at least it should isolate chunks that can be understood with little effort. Having to understand 1000 lines of code before being reasonably certain you can make a change is far harder than understanding some 20-50 lines.
    - hombre_fatal 4y ago
      On the other hand, since you don’t know future requirements, it’s easier to refactor/abstract unabstracted code than to refactor/reabstract already-/wrongly-abstracted code. Function extraction adds indirection that doesn’t necessarily pay its own rent. It’s not free.
    - usrbinbash 4y ago
      If requirements change next week, I change the code ... next week. I don't change code now to be prepared for something that may or may not happen. > but at least it should isolate chunks that can be understood with little effort. The chunks are easier to understand. But the laundry list function that calls them isn't because it's logic is now spread over 10 different functions.
      - 4y ago
greenthrow 4y ago
Uhhh if you have a 1,000 line function it definitely should be broken down. Even a 100 line function is borderline. It's gonna be nigh impossible to write comprehensive unit tests for a 1,000 line function.
- mgkimsal 4y ago
  I had a method around... 700-800 lines. I extracted a bunch of things in to smaller bits, and the resulting block was around 140 lines, with generally understandable names. It's 'doing a lot' because... well, we have a complex block of data with many bits that need to be pulled in based on some logic. Someone new started and... within 10 minutes commented in the code "150 lines is way too long, we need to understand why this is so bad and how to refactor this to be good - you should never do so much in a method". Could I break it up even more? And have one method that composes 4 then each compose 4 that each compose 3? Possibly. But there's a tradeoff, and... what annoyed me more was that there was 1) no questions asked, 2) no view of the history of how it went from 800 down to 150, 3) no review of the tests in place which document and demonstrate that it does, in fact, work. Pursuing brevity at all costs has a higher price than some people understand. Worked with the person a while longer, and they eschewed tests because... "hey, everything I write is already so small and understandable, there's no need for tests", demonstrating a pretty basic misunderstanding of tests (imo).
  - avgDev 4y ago
    Anyone new that immediately shits on a codebase is a huge red flag. I have seen 1500 method in an app I was rewriting. The dev no longer worked for us. It was badly written, I told the management it wasn't optimal and there were some issues with it but I said I don't know under what constraints the dev was working. I simply wasn't there when it was written and have no idea if it was incompetence or some other factors. The point is anyone who thinks certain way is BAD and needs to be immediately improved while barely knowing the code base is a bit incompetent. I honestly would wait weeks to start bringing up some issues I may see in the code base or sql. I would learn the team dynamics first and try to see if there is any other reason for those things.
mannykannot 4y ago
The fundamental problem here is that it is done through syntactic rather than semantic analysis.
Supermancho 4y ago
> A 1000 line function broken down into 100 different 10 line functions which are then each called in turn (and need lots of parameters to pass around the inevitable shared state) is actually often less readable than just a 1000 line function. Aside from the 1000 line function (sheesh), this is a failure of the IDE, not of the practice. We have computers that can inline these functions so you could see the code in a big 1000 line thunk if you would like, but only a LISP IDE gives you inlining afaik. > “Good code is simple” doesn’t actually say anything. That's a bold claim and I will assert it is wholly incorrect. Good Code is simple and simple code is VERBOSE > whether that translates to “simple” code depends on the programmers. That is incorrect. It largely depends on the language abstractions. When the abstractions are familiar, it goes from incomprehensible (literally) to familiar. Code cannot seem simple if you do not have the familiarity with the abstractions used. > Should we strive to satisfy the Shakespeare for Dummies demographic Yes. > Programmers seem to believe in a realm of beautiful, readable, easy-to-maintain code that they haven’t seen or worked with yet Lots of programmers believe it because they have seen it. I have. It was embedded in hard-to-maintain code, but it was there.
- tremon 4y ago
  Good Code is simple and simple code is VERBOSE It doesn't mean anything because both good and simple are subject to on-the-spot redefinition. At what skill level does code become "simple"? Does all code have to be ELI5 to be good? When a Node developer looks at CUDA implementation of k-means regression analysis and finds it "difficult", does that mean the code isn't good? When the abstractions are familiar So you're saying it does depend on the programmer, because the programmer should be familiar with the abstractions used?
  - b3morales 4y ago
    In case you have not seen it, there is an excellent talk by Rich Hickey on what "simple" is and how it differs from "familiar" or "easy": https://www.infoq.com/presentations/Simple-Made-Easy https://www.infoq.com/presentations/Simple-Made-Easy He proposes that "simple" is more objective than subjective.
  - 4y ago
rendaw 4y ago
Many languages have ways to reduce scope in a single function (rust: drop, c/c++ java: {}, etc) so you can have a long function that means "these things need to happen in sequence" without suffering from excess shared state. Though obviously not perfect.
- sophacles 4y ago
  In rust you can also do something like: let (x, y) = { ... (some code that only serves to create x and y) }; I really really like that feature for controlling scoping - it's like a single use function that doesn't involve so much navigation. Other languages allow similar constructs with blocks too. For ones that allow inline lambdas, you can also get similar results by defining and immediately calling the function.

SemanticStrengh 4y ago

This. A related term being ravioli code.

danuker 4y ago

For what it's worth, my take on it is that you should extract whatever bits of pure functions you can find in the 1000-line ones. This would help testing and understanding. You can test and understand the parts independently, and only have to test the huge function in order to check that the parts are attached correctly. https://rhodesmill.org/brandon/talks/#hoist https://rhodesmill.org/brandon/talks/#hoist https://danuker.go.ro/the-grand-unified-theory-of-software-architecture.html https://danuker.go.ro/the-grand-unified-theory-of-software-a...

andi999 4y ago
Only if the pure function have a few parameters and do not need to carry the world.
- danuker 4y ago
  You can bundle the parameters into plain/pure/humble objects where it makes sense. https://khanlou.com/2014/12/pure-objects/ https://khanlou.com/2014/12/pure-objects/ http://xunitpatterns.com/Humble%20Object.html http://xunitpatterns.com/Humble%20Object.html
  - mannykannot 4y ago
    'Where it makes sense' is key - otherwise you are just stuffing the wiring behind the desk, adding to the difficulty of understanding it.
marginalia_nu 4y ago
This a bit of an aside, but I wonder if people would be less prone to advocate pure functions if they had a name that didn't sound like a paladin had come up with them. Would people as eagerly advocate rewriting code to break it into greasy functions?
- danuker 4y ago
  "Side-effect-free" would be a mouthful, and I don't know any other name that is both accurate and well-known.
  - marginalia_nu 4y ago
    Well pure is just as arbitrary as greasy, isn't it? Except one has positive connotations and the other has negative ones.
Verdex 4y ago
Well not necessarily just 'pure'. You also want unsurprising. For example imagine a pure function that takes a string and returns a string. The returned string has every character lower cased. Okay name it to_lower and move it out. But now imagine that it returns the complete text of Shakespeare if the input it 'bob' (why? customer requirement. their business literally falls apart if this doesn't happen). In this case you probably shouldn't pull it out into it's own thing because it's so specific (and weird) that you'll want it nearby it's use case (and not available for developers to accidentally use for the non Shakespeare uses). * yeah this is convoluted. There are better ways to factor such a function. However the point is that something can be 'pure' but still not a good candidate to be pulled out of context.
- dolni 4y ago
  > In this case you probably shouldn't pull it out into it's own thing because it's so specific (and weird) that you'll want it nearby it's use case (and not available for developers to accidentally use for the non Shakespeare uses). I guess it depends on how much this weird Shakespearean to_lower is used around the codebase. If there is one spot where it's used, I would probably factor out to_lower and add a special case check for "bob" in the business logic code (outside the scope of to_lower). If the Shakespearean to_lower gets used in various places it actually might make sense to be factored into its own function.
  - Verdex 4y ago
    * yeah this is convoluted. There are better ways to factor such a function. However the point is that something can be 'pure' but still not a good candidate to be pulled out of context.
usrbinbash 4y ago
> For what it's worth, my take on it is that you should extract whatever bits of pure functions you can find in the 1000-line ones. Do these bits of pure functions appear anywhere else in the code? If so, sure, I happily pull them out into their own functions. If not, I will leave them exactly where they are. It's easier to read a laundry list than to look up many different parts in isolation without context. As for testability: If some bit of code only appears in that one long laundry-list function, what do I gain by testing it outside of that context?
- Dudeman112 4y ago
  That's acceptable if you have really good reasons to assume the requirements won't change next week, which happens almost never. 1000x1 lines of code all at once have a lot of space for hidden non-obvious interactions. Maybe it won't have that, but I haven't ever encountered it. Making it 10x100 lines of code won't necessarily make it better, but at least it should isolate chunks that can be understood with little effort. Having to understand 1000 lines of code before being reasonably certain you can make a change is far harder than understanding some 20-50 lines.
  - hombre_fatal 4y ago
    On the other hand, since you don’t know future requirements, it’s easier to refactor/abstract unabstracted code than to refactor/reabstract already-/wrongly-abstracted code. Function extraction adds indirection that doesn’t necessarily pay its own rent. It’s not free.
  - usrbinbash 4y ago
    If requirements change next week, I change the code ... next week. I don't change code now to be prepared for something that may or may not happen. > but at least it should isolate chunks that can be understood with little effort. The chunks are easier to understand. But the laundry list function that calls them isn't because it's logic is now spread over 10 different functions.
    - 4y ago

greenthrow 4y ago

Uhhh if you have a 1,000 line function it definitely should be broken down. Even a 100 line function is borderline. It's gonna be nigh impossible to write comprehensive unit tests for a 1,000 line function.

mgkimsal 4y ago
I had a method around... 700-800 lines. I extracted a bunch of things in to smaller bits, and the resulting block was around 140 lines, with generally understandable names. It's 'doing a lot' because... well, we have a complex block of data with many bits that need to be pulled in based on some logic. Someone new started and... within 10 minutes commented in the code "150 lines is way too long, we need to understand why this is so bad and how to refactor this to be good - you should never do so much in a method". Could I break it up even more? And have one method that composes 4 then each compose 4 that each compose 3? Possibly. But there's a tradeoff, and... what annoyed me more was that there was 1) no questions asked, 2) no view of the history of how it went from 800 down to 150, 3) no review of the tests in place which document and demonstrate that it does, in fact, work. Pursuing brevity at all costs has a higher price than some people understand. Worked with the person a while longer, and they eschewed tests because... "hey, everything I write is already so small and understandable, there's no need for tests", demonstrating a pretty basic misunderstanding of tests (imo).
- avgDev 4y ago
  Anyone new that immediately shits on a codebase is a huge red flag. I have seen 1500 method in an app I was rewriting. The dev no longer worked for us. It was badly written, I told the management it wasn't optimal and there were some issues with it but I said I don't know under what constraints the dev was working. I simply wasn't there when it was written and have no idea if it was incompetence or some other factors. The point is anyone who thinks certain way is BAD and needs to be immediately improved while barely knowing the code base is a bit incompetent. I honestly would wait weeks to start bringing up some issues I may see in the code base or sql. I would learn the team dynamics first and try to see if there is any other reason for those things.

mannykannot 4y ago

The fundamental problem here is that it is done through syntactic rather than semantic analysis.

Supermancho 4y ago

> A 1000 line function broken down into 100 different 10 line functions which are then each called in turn (and need lots of parameters to pass around the inevitable shared state) is actually often less readable than just a 1000 line function. Aside from the 1000 line function (sheesh), this is a failure of the IDE, not of the practice. We have computers that can inline these functions so you could see the code in a big 1000 line thunk if you would like, but only a LISP IDE gives you inlining afaik. > “Good code is simple” doesn’t actually say anything. That's a bold claim and I will assert it is wholly incorrect. Good Code is simple and simple code is VERBOSE > whether that translates to “simple” code depends on the programmers. That is incorrect. It largely depends on the language abstractions. When the abstractions are familiar, it goes from incomprehensible (literally) to familiar. Code cannot seem simple if you do not have the familiarity with the abstractions used. > Should we strive to satisfy the Shakespeare for Dummies demographic Yes. > Programmers seem to believe in a realm of beautiful, readable, easy-to-maintain code that they haven’t seen or worked with yet Lots of programmers believe it because they have seen it. I have. It was embedded in hard-to-maintain code, but it was there.

tremon 4y ago
Good Code is simple and simple code is VERBOSE It doesn't mean anything because both good and simple are subject to on-the-spot redefinition. At what skill level does code become "simple"? Does all code have to be ELI5 to be good? When a Node developer looks at CUDA implementation of k-means regression analysis and finds it "difficult", does that mean the code isn't good? When the abstractions are familiar So you're saying it does depend on the programmer, because the programmer should be familiar with the abstractions used?
- b3morales 4y ago
  In case you have not seen it, there is an excellent talk by Rich Hickey on what "simple" is and how it differs from "familiar" or "easy": https://www.infoq.com/presentations/Simple-Made-Easy https://www.infoq.com/presentations/Simple-Made-Easy He proposes that "simple" is more objective than subjective.
- 4y ago

rendaw 4y ago

Many languages have ways to reduce scope in a single function (rust: drop, c/c++ java: {}, etc) so you can have a long function that means "these things need to happen in sequence" without suffering from excess shared state. Though obviously not perfect.

sophacles 4y ago
In rust you can also do something like: let (x, y) = { ... (some code that only serves to create x and y) }; I really really like that feature for controlling scoping - it's like a single use function that doesn't involve so much navigation. Other languages allow similar constructs with blocks too. For ones that allow inline lambdas, you can also get similar results by defining and immediately calling the function.