4 ms·

As long as the function is doing one thing conceptually, I don't see the benefit of breaking it up into multiple routines just because it's too long (for some a

by spc476 11y ago

As long as the function is doing one thing conceptually, I don't see the benefit of breaking it up into multiple routines just because it's too long (for some arbitrary length). Not only do you have the cognitive overhead of trying to come up with a name [1], but passing in all the required parameters to the function. And for this function, I personally don't see any good "breaking points". At work, I have to deal with a code base where code is split up like that, and it's a royal pain to follow, and the resulting functions aren't usable by anything else. [1] The two hardest problems in computer science is cache invalidation, naming things, and off by one errors.

alkonaut 11y ago
It's true that a function in an expressive language can be 10 lines, which in C is 70 lines just because of verbosity (repeated assigns instead of constructors and so on). So while the threshold is certainly higher in C, this is still too large. It does 3-4 things conceptually but 2 (lookups) are already broken out. I can see at least one opportunity more for extracting a function. It's important not least because of testing.
- spc476 11y ago
  I'd like to know what you would extract out to its own function, because I certainly don't seen anything worth making its own function.
  - alkonaut 11y ago
    Maybe it's just my aversion to C (and procedural) in general but I think assigning 3 fields of the same struct in succession as a sign that you are making an inline constructor that should be explicit. The chunk I meant looked like a possibility though was the hint bit. The point of smaller code is testability. It's worth even making it somewhat harder to read. If a test makes three things in succession that in their own functions would have required 3 tests each, the aggregated do-everything function requires much more than the 9 test cases. So dividing the method in two, even if arbitrary and bad for readability can have a positive impact since the total code volume is reduced a lot. All of this ^ only applies if the code actually has tests. If not - then don't divide arbitrarily if it reduces readability at all. But then again without tests you have bigger problems than readability...
    - spc476 11y ago
      It's your aversion to C, because the hint thing does not do what you think it does. It doesn't assign three fields of a structure, it pulls three fields out of a structure (and in my opinion, making a function just to do that is silly---it's not an opaque object (as it's documented in man pages as to the fields it has), hides the intent and further more, clutters up the code. If you make this "accessor" function static [1], then you have to include the test code in the same C file (cluttering that file with code only used for testing [2]) or you clutter the C global function namespace with a function that's quite possibly only used on one location (and thus, can't be inlined at all due to C semantics---just some of the realities of attempting TDD in C). [1] Functions and variable marked as "static" are only visible in the file. [2] And you end up with pretty much dead code in a release version. You could exclude such code from a release build, but then you can't actually test the release build of each function.
      - alkonaut 11y ago
        I used to think C was bad without even considering the lacking modules/namespaces etc and what it means for testing. It does encourage few and large hard-to-test public functions. Am I right in suspecting that the test suites of libs like these are pretty small?
        spc476 11y ago
        I think it really depends upon the codebase. Libraries are easier to test than applications in general, since you have a published API to throw at testing. One approach (and it seems to work for LuaJIT, a mid-sized C based project) is to just collect test cases as bugs arise (create the smallest input possible to trigger the bug in question) and as changes are made, rerun all those cases as a type of ever-growing regression test. Or, you know, run a fuzzer.

by spc476 11y ago

alkonaut 11y ago

It's true that a function in an expressive language can be 10 lines, which in C is 70 lines just because of verbosity (repeated assigns instead of constructors and so on). So while the threshold is certainly higher in C, this is still too large. It does 3-4 things conceptually but 2 (lookups) are already broken out. I can see at least one opportunity more for extracting a function. It's important not least because of testing.

spc476 11y ago
I'd like to know what you would extract out to its own function, because I certainly don't seen anything worth making its own function.
- alkonaut 11y ago
  Maybe it's just my aversion to C (and procedural) in general but I think assigning 3 fields of the same struct in succession as a sign that you are making an inline constructor that should be explicit. The chunk I meant looked like a possibility though was the hint bit. The point of smaller code is testability. It's worth even making it somewhat harder to read. If a test makes three things in succession that in their own functions would have required 3 tests each, the aggregated do-everything function requires much more than the 9 test cases. So dividing the method in two, even if arbitrary and bad for readability can have a positive impact since the total code volume is reduced a lot. All of this ^ only applies if the code actually has tests. If not - then don't divide arbitrarily if it reduces readability at all. But then again without tests you have bigger problems than readability...
  - spc476 11y ago
    It's your aversion to C, because the hint thing does not do what you think it does. It doesn't assign three fields of a structure, it pulls three fields out of a structure (and in my opinion, making a function just to do that is silly---it's not an opaque object (as it's documented in man pages as to the fields it has), hides the intent and further more, clutters up the code. If you make this "accessor" function static [1], then you have to include the test code in the same C file (cluttering that file with code only used for testing [2]) or you clutter the C global function namespace with a function that's quite possibly only used on one location (and thus, can't be inlined at all due to C semantics---just some of the realities of attempting TDD in C). [1] Functions and variable marked as "static" are only visible in the file. [2] And you end up with pretty much dead code in a release version. You could exclude such code from a release build, but then you can't actually test the release build of each function.
    - alkonaut 11y ago
      I used to think C was bad without even considering the lacking modules/namespaces etc and what it means for testing. It does encourage few and large hard-to-test public functions. Am I right in suspecting that the test suites of libs like these are pretty small?
      - spc476 11y ago
        I think it really depends upon the codebase. Libraries are easier to test than applications in general, since you have a published API to throw at testing. One approach (and it seems to work for LuaJIT, a mid-sized C based project) is to just collect test cases as bugs arise (create the smallest input possible to trigger the bug in question) and as changes are made, rerun all those cases as a type of ever-growing regression test. Or, you know, run a fuzzer.