6 ms·

Have you ever seen a well structured 1000 line function? I'm sure they exist - maybe some sort of exceedingly complicated data transform or something. But in a

Have you ever seen a well structured 1000 line function? I'm sure they exist - maybe some sort of exceedingly complicated data transform or something. But in almost every situation I've seen, a 1000 line function has countless side effects, probably sets a few globals, takes loads of poorly named arguments, each of which is a nested data structure which it reaches deeply into and often has the same for loop copied and pasted 10 times with one character changed. Often a 1000 line function is actually 5 or 6 20 line functions. I'm sure there are legitimate exceptions, but I've never seen them.

deelly 3y ago
I`m not OP, but yes, I saw. Thats my personal opinion, and nothing more: Something like complex one time financial/workflow/maintenance operation that includes calling dozens of different smaller functions, but very well structured. It does not make sense to separate it more into different functions, because execution is generally linear and having to deal with tree of calls where some calls is depends on state of the previous is become cumbersome and makes reading and making changes more complex. Again, thats my personal feeling, and nothing more.
- deleted 3y ago
  [deleted]
Thorrez 3y ago
https://github.com/github/putty/blob/master/terminal.c#L3281 https://github.com/github/putty/blob/master/terminal.c#L3281 This function is 1830 lines long. It's reasonably well structured I think. Although the #if 0 are maybe not so good.
- mattlondon 3y ago
  A lot of those if-/case-blocks are precisely where I'd put functions :) If you changed a bunch of those to separate, pure (i.e. side-effect-free) functions it would if nothing else make unit testing a breeze, and then you'd be free to fix bugs in the logic without fear. As it is, if I had a bug in that huge function I'd be really worried about breaking some edge-condition or implied-state 500 lines up etc.
  - fingerlocks 3y ago
    They can’t be side effect free, that’s the point. The switch statements are mutating the input.
    - Turskarama 3y ago
      There's a lot that can be, take this for instance (line 3336): if (c == '\033') term->print_state = 1; else if (c == (unsigned char)'\233') term->print_state = 2; else if (c == '[' && term->print_state == 1) term->print_state = 2; else if (c == '4' && term->print_state == 2) term->print_state = 3; else if (c == 'i' && term->print_state == 3) term->print_state = 4; else term->print_state = 0; This could be turned into a pure function that takes c and print_state as input, and returns a new print_state which the outer function assigns. That's 12 lines turned into 1. term->print_state = newState(c, term->print_state); (I am not a C developer so the sytax could be wrong). Just because the outer function is impure doesn't mean it can't in turn call pure functions.
      - BenFrantzDale 3y ago
        I totally agree. And you can always write mutating “functions” (what get called “algorithms” in the C++ world), like C++’s `std::ranges::sort(range)`.
      - couchand 3y ago
        It's a Mealy state machine, it should be encoded as a state machine! Why some folks will absolutely insist that a series of manually-written case statements in a single thousand line function is the epitome of style, when there's a pure state machine model inside wishing you would free it from the shackles of if else if else if else....
        Turskarama 3y ago
        I have some experience with state machines, and I absolutely hate them. It's so easy to get lost, you never know where a function will go to next. There's a reason stacks are ubiquitous, they're much easier to fit in your head.
- pjc50 3y ago
  Well that's horrendous. Sorry, Simon. Each of those big "switch" statements should be broken out.
  - PeterisP 3y ago
    Why? That would create lots of small functions that each is called only once from only one place in the code and still are extremely coupled to the invoking switch statement, often having multiple inputs and multiple outputs which then the invoking statement needs to write in the proper places to which the function wouldn't have access. Breaking out functions makes sense when you get either reuse or decoupling, but in this case you don't get any of these.
    - ilaksh 3y ago
      Because it would objectively and shockingly obviously make the code easier to understand. You would be able to see what it does at a high level at a glance and then drill down into the functions and sub-functions to focus on a particular part of it. With this version, to get the high level overview you have to scroll through multiple pages of code and find the comments that say what each section does.
      - nickitolas 3y ago
        So the problem is being able to drill down? Most decent IDEs allow you to collapse code blocks (i.e brackets) if that's what you want to do, and the comments accomplish the same thing a function name would. Some editors support region comments that start out collapsed and can be named. I don't see why separate functions would be anyones first instinct to reach for here.
        ilaksh 3y ago
        I don't see why anyone's first instinct to reach for would be relying on some editor's ability to optionally collapse code blocks. What happens if someone wants to use a different editor that doesn't have that feature or doesn't display it quite right anymore? Lol.
- bombela 3y ago
  Oh my God. The sneaky `else` at line 3400. edit: another one at 3826 with a preprocessor define interleaved.

deelly 3y ago

I`m not OP, but yes, I saw. Thats my personal opinion, and nothing more: Something like complex one time financial/workflow/maintenance operation that includes calling dozens of different smaller functions, but very well structured. It does not make sense to separate it more into different functions, because execution is generally linear and having to deal with tree of calls where some calls is depends on state of the previous is become cumbersome and makes reading and making changes more complex. Again, thats my personal feeling, and nothing more.

deleted 3y ago
[deleted]

Thorrez 3y ago

https://github.com/github/putty/blob/master/terminal.c#L3281 https://github.com/github/putty/blob/master/terminal.c#L3281 This function is 1830 lines long. It's reasonably well structured I think. Although the #if 0 are maybe not so good.

mattlondon 3y ago
A lot of those if-/case-blocks are precisely where I'd put functions :) If you changed a bunch of those to separate, pure (i.e. side-effect-free) functions it would if nothing else make unit testing a breeze, and then you'd be free to fix bugs in the logic without fear. As it is, if I had a bug in that huge function I'd be really worried about breaking some edge-condition or implied-state 500 lines up etc.
- fingerlocks 3y ago
  They can’t be side effect free, that’s the point. The switch statements are mutating the input.
  - Turskarama 3y ago
    There's a lot that can be, take this for instance (line 3336): if (c == '\033') term->print_state = 1; else if (c == (unsigned char)'\233') term->print_state = 2; else if (c == '[' && term->print_state == 1) term->print_state = 2; else if (c == '4' && term->print_state == 2) term->print_state = 3; else if (c == 'i' && term->print_state == 3) term->print_state = 4; else term->print_state = 0; This could be turned into a pure function that takes c and print_state as input, and returns a new print_state which the outer function assigns. That's 12 lines turned into 1. term->print_state = newState(c, term->print_state); (I am not a C developer so the sytax could be wrong). Just because the outer function is impure doesn't mean it can't in turn call pure functions.
    - BenFrantzDale 3y ago
      I totally agree. And you can always write mutating “functions” (what get called “algorithms” in the C++ world), like C++’s `std::ranges::sort(range)`.
    - couchand 3y ago
      It's a Mealy state machine, it should be encoded as a state machine! Why some folks will absolutely insist that a series of manually-written case statements in a single thousand line function is the epitome of style, when there's a pure state machine model inside wishing you would free it from the shackles of if else if else if else....
      - Turskarama 3y ago
        I have some experience with state machines, and I absolutely hate them. It's so easy to get lost, you never know where a function will go to next. There's a reason stacks are ubiquitous, they're much easier to fit in your head.
pjc50 3y ago
Well that's horrendous. Sorry, Simon. Each of those big "switch" statements should be broken out.
- PeterisP 3y ago
  Why? That would create lots of small functions that each is called only once from only one place in the code and still are extremely coupled to the invoking switch statement, often having multiple inputs and multiple outputs which then the invoking statement needs to write in the proper places to which the function wouldn't have access. Breaking out functions makes sense when you get either reuse or decoupling, but in this case you don't get any of these.
  - ilaksh 3y ago
    Because it would objectively and shockingly obviously make the code easier to understand. You would be able to see what it does at a high level at a glance and then drill down into the functions and sub-functions to focus on a particular part of it. With this version, to get the high level overview you have to scroll through multiple pages of code and find the comments that say what each section does.
    - nickitolas 3y ago
      So the problem is being able to drill down? Most decent IDEs allow you to collapse code blocks (i.e brackets) if that's what you want to do, and the comments accomplish the same thing a function name would. Some editors support region comments that start out collapsed and can be named. I don't see why separate functions would be anyones first instinct to reach for here.
      - ilaksh 3y ago
        I don't see why anyone's first instinct to reach for would be relying on some editor's ability to optionally collapse code blocks. What happens if someone wants to use a different editor that doesn't have that feature or doesn't display it quite right anymore? Lol.
bombela 3y ago
Oh my God. The sneaky `else` at line 3400. edit: another one at 3826 with a preprocessor define interleaved.