Timeline for How should I react when a co-worker says his 3000 line method is optimized? Should I report it to my boss?
Current License: CC BY-SA 4.0
20 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Feb 12, 2019 at 23:12 | comment | added | Adam Barnes | @SimonRichter In that exact case, I would model it as a chain. You construct the chain with a context and the list of functions itself, and then enact the chain, which calls each function in turn with the context and the results of the previous function. | |
| Feb 11, 2019 at 15:26 | comment | added | Simon Richter | @Darkwing, my point is that this isn't always possible. I totally agree that it usually is, but the important question isn't "is this function too long?" but "is this function maintainable?" The function I was thinking about when I wrote my answer deals with DSP chain timings in a fixed IC, it's long because the DSP chain is long, and every part depends on the previous one, so I'd have to pass lots of state around if I wanted to make multiple functions. I could split it up by creating a structure with all my local variables and iterating through a list of functions to modify it, but why? | |
| Feb 11, 2019 at 14:25 | comment | added | Frank Hopkins | @AdamBarnes then your problem is stupid function naming, the equivalent would be the 1000 line function be named "f". Sensibly you split the 1000 line "deletePrivacyData" function into little functions, even if there is no code duplication like "deleteCookies", "deleteHistory", "deleteCachedData". | |
| Feb 11, 2019 at 1:01 | comment | added | Adam Barnes |
@RoyTinker Indirection and delegation only make things harder to understand if you need to understand all their workings. Calling a function named f() is MUCH harder to read than delete_browser_history(), because one needs to go and read the contents of f(). The point of such code splitting is to save the reader from having to think about how it works.
|
|
| Feb 10, 2019 at 15:36 | comment | added | dan-klasson | That this answer has 30 upvotes is concerning. | |
| Feb 10, 2019 at 14:10 | comment | added | Matteo Italia | When reading/debugging some one else's code, I'll take a single gigantic method over yo-yo code where I have to navigate between AbstractModulatorFactoryAdapters (probably used only once in the codebase) any day; this Carmack's piece is also particularly illuminating. | |
| Feb 9, 2019 at 20:06 | comment | added | SafeFastExpressive | A thousand line switch statement doesn't do anything except make a single decision. That has nothing to do with OPs problem, which is 3,000 lines of actual working code, not a switch statement. | |
| Feb 9, 2019 at 14:17 | comment | added | DaveG | @Mark The only time I've seen single gigantic switch statement is from generated code, like yacc output. In which case I never actually looked at the switch, I just modified the grammar. I inherited a bunch of crummy code recently and have been breaking apart, rewriting and in some cases completely throwing away some huge functions and classes, and in every case the result is smaller, cleaner, easier to use. | |
| Feb 9, 2019 at 11:15 | comment | added | spectras | I have never seen a 1000+ line function that couldn't be made much better by splitting up functionality. It violates most of the quality standards of the industry. In the 70s there were justifications because tools were lacking. Nowadays, it's just awful code quality and the kind of technical debt that kills businesses. | |
| Feb 9, 2019 at 6:17 | comment | added | David | Interesting comment. Does anyone have an example of 1000 lines of code in a single function that couldn't be refactored into more readable and maintainable code? Maybe there are, but I can't think of any possible need for such a horrible function. I just Googled C# to check it had classes (as I thought it did); it's not like you're locked into using globals to compensate for a horrible language... Perhaps the original poster could find one or two specific examples of things that could be refactored and then start from there? | |
| Feb 9, 2019 at 4:38 | comment | added | jpmc26 | @Dancrumb Simon isn't saying or implying there's a dichotomy. He's saying that dogmatically insisting on short functions can also lead to poor code. Whether it's long or short is the wrong question to ask. The question to ask is, "Does the function provide a good abstraction?" And while that has a correlation with length, it is independent of length. Use length as a heuristic to trigger more careful review, rather than being dogmatic about it. | |
| Feb 9, 2019 at 3:13 | comment | added | Loren Pechtel | @ajacian81 I've actually seen 1k+ of code that did one thing. It was a massive switch. In modern times it would most likely be handled in a more object-oriented fashion but this was in the 80s. I've also gone into several hundred lines worth of switch in reading a config file. | |
| Feb 8, 2019 at 23:34 | comment | added | Mark | @DaveG, single gigantic switch statement that does event dispatch. Yes, I suppose it could have been broken up into groups of related events or something, but 250+ nearly identical four-line blocks of code sorted alphabetically by event name is easy enough to understand. | |
| Feb 8, 2019 at 22:56 | comment | added | Graham | @Dancrumb It entirely depends on the function. If it's spaghetti code, sure. But if it's a state machine, then please do make it one 1000-line function, especially if you've got diagrams for it too. We can all read that, and maintain it. My horror is the CS design-pattern "expert" fresh from uni who wants to split that into a dozen interconnected files and functors and god knows what, just because hey, design patterns. No. No, no, no. No. Nope. Hell no. | |
| Feb 8, 2019 at 22:45 | comment | added | Dancrumb | I think that's something of a false dichotomy. Refactoring a 1000+ line function does not necessitate vast levels of indirection. Also, there really is no way a 1000+ line function could not be sensibly broken up into at least a handful of smaller functions that encapsulate coherent sections of the larger. | |
| Feb 8, 2019 at 22:16 | comment | added | Roy Tinker | @DaveG Sounds like you haven't yet encountered the 10-levels-deep indirection spread across 50 files alternative. Sometimes a readable, well-named 1000-line function can evoke a sigh of relief that it didn't go heavy on indirection. This is especially true when it's implementing a complex, multi-step workflow. (That's not to say that careful abstraction wouldn't have been a somewhat better alternative.) | |
| Feb 8, 2019 at 20:48 | comment | added | ajacian81 | There's no way a 1k line function does one thing. | |
| Feb 8, 2019 at 19:56 | comment | added | Sloloem | A 3k line function is effectively untestable. A 1k line function probably is as well unless it does literally ONE thing. An automated test suite needs to be pitched to a non-technical boss on the merits of increasing production stability which reduces downtime and customer complaints which frees up time to develop more functionality. Less downtime, happier customers, and speedier development of new features == more money. | |
| Feb 8, 2019 at 17:46 | comment | added | DaveG | I really disagree that any 1,000 line function is going to be clearer and more readable than the equivalent broken up into smaller functions. I find it really, really unlikely that a 1,000 function doesn't have duplicated blocks of code that could be pulled out into functions. I really don't want to read 1,000 lines to figure out what a function does. | |
| Feb 8, 2019 at 16:03 | history | answered | Simon Richter | CC BY-SA 4.0 |