5 ms·
>I get the cold sweats when I see methods longer than a few lines of code This guys code might not be as readable as he thinks. I do not want to read through y
by thatguyknows 3y ago
>I get the cold sweats when I see methods longer than a few lines of code
This guys code might not be as readable as he thinks. I do not want to read through your hundreds of 10 line functions because a few people said "long functions bad".
I liked the portion on naming. My God the amount of AbstractFactorySingletonViewModel garbage I see is astounding.
- mberning 3y agoIt’s a different kind of overhead. But I will take the tradeoff between ten 10 line methods and one 100 line method every time. I stand a much better chance of successfully modifying and testing the former.
- pxc 3y agoSmall functions are easy enough for me to deal with when they're basically library functions, but when they're split up/distributed in an OOP-y way I find navigating them and keeping track of them way more troublesome.
- mattwad 3y agoEspecially if some of them have side-effects, which is often the case unless you're dealing with a purely functional codebase which is pretty rare.
- RSHEPP 3y agoI think this is one of the more important distinctions, but you never seen it brought up. High level, business oriented functions almost never end up being more readable split up into a bunch of sub functions.
- vineyardmike 3y agoMy second job had amazing code quality, and one that that always struck me was how the code was separated cleanly into layers that made the business logic layer super obvious. It was 3 layers - API interface (unpacking objects, etc) - Business Layer - DB/System Interface. The business logic changes could often be totally contained, without updating the API or the database. It would be so obvious what layer needed to be edited, and the problems could be fixed so much faster. Rarely did a commit change two layers at once. You can easily handle AuthN in the API layer, AuthZ in the business layer, and then access to a db in the system layer. Assume API is un-authZ’ed, assume the system layer is fully AuthZ’ed, etc.
- Kamq 3y agoThat's fair if you only have 100 lines of code, but I don't think it automatically follows that the best solution for 10,000 lines of code is 1,000 ten line functions. There's a tradeoff between understanding the function itself and understanding how/where the function is used (and therefore understanding what will break when that function is changed). The best heuristic I've got at the moment is roughly the square root of the size of the module. So, for the 10,000 line module, my vague instinct is that I'll have one hundred functions approximately one hundred lines each. That's not a hard and fast rule, just an observation about tradeoffs and I would definitely expect (quite possibly even most) functions to differ significantly without losing sleep over it.
- rootlocus 3y agoSimple anecdote: A colleague of mine had a really hard time getting some topologic sort to work. I kept finding issues with his implementations and he was getting frustrated so we aggreed to have a pair programming session. He wanted to put all the code in a single method (not terribly long, maybe 30, 40 lines), but the conditions kept nesting and no matter how much he tried to figure them out, he couldn't get them to work. I convinced him to extract two methods which would only be used once and only inside this sorting method. He was vehemently against it. After he finally agreed he realized the code was so easy to read, it was basically self-explanatory and obviously correct. No ammount of fiddling with the if statements could've produced that. I don't impose fixed line counts for my methods. But when it makes sense, the readability improves a lot.
- CameronNemo 3y agoYeah line count is a hard metric to evaluate. Is your code 100 lines of code like this: let foo = Foo::new() .with_bar() .with_baz(bak, bat) .listen(); Or code like this: for f in in foo.iter() { if f == 1 || f > 9 || f % 2 == 0 || is_edge_case(f) { ret.push((f * 100).to_string()); } } Makes a big difference for readability. If you have a complicated conditional, go ahead and give it a name!
- MattPalmer1086 3y agoI have essentially the same story, so I won't repeat it. The big win was when my colleague seized on the idea of self documenting code. That was all he needed to write better structured code.
- gspencley 3y agoSmall functions are not about readable code. It's about single responsibility. Single responsibility is an engineering principle that is not limited to software development. It is the reason that we don't combine the breaking functionality in our car with the am/fm radio. We keep moving parts isolated because the fewer "things" that something does, the less likely it is to break. The less complicated it is. But there are other side effects to creating small, single-purpose functions (or anything for that matter). It is not always obvious when you will have an opportunity to reuse something. And duplication is not always apparent. When you take single-responsibility as far as you can go, you not only isolate all of your moving parts but you maximize the opportunities for reuse. And it goes even further than that. Your large functions likely have a few dependencies, at least. Those dependencies will make your functions more difficult to write tests for. And your test cases will be more complicated if you have larger functions because they are doing more than one thing that needs to be captured. Readability is more about expressing the intent of code. You can do that in "long functions." You can express the intent of code while making it extremely compilcated. Hell, just add lots of verbose comments and your code will be more "readable." Since you seem to be one of those people who has a stick up you about design patterns (your last comment about AstractFactorySingletonViewModel ... who hurt you?) I will offer you this piece of food for thought: the purpose of "best practices" and design patterns is to SIMPLIFY code. If you ever see a misapplication of them in the wild* then what you are witnessing is not "over" engineering ... it is POOR engineering. Consider that before throwing the baby out with the bathwater. * I rarely do, so I often wonder to myself if this is a made up problem by lazy devs who don't want to actually study theory. But I do hear that this occurs from time to so I'll take you at your word that there are people out there that don't know how and when to actually apply design patterns properly. The problem is the misapplication, not the patterns themselves - which are just common solutions to recurring problems. Do you not think DRY is a good idea?
- thatguyknows 3y agoSome links you may find useful https://copyconstruct.medium.com/small-functions-considered-harmful-91035d316c29 https://copyconstruct.medium.com/small-functions-considered-... https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpriseEdition https://github.com/EnterpriseQualityCoding/FizzBuzzEnterpris...
- bazoom42 3y agoAgree - if a function is otherwise cohesive and doesnt mix independent concerns, I prefer one 100 line fuction to 25 4-line functions. Splitting functions is tempting because it makes each individual function seem simper, but the higher cost in complexity and maintainability is not as directly visible. Shorter functions are fine when appropriate, but length by itself is not an indicator of quality.