5 ms·
He missed at least one: functions should be "small" and broken up for "testability." No, functions should perform one task and perform it well. Not every condi
by sidlls 10y ago
He missed at least one: functions should be "small" and broken up for "testability."
No, functions should perform one task and perform it well. Not every conditional test inside the function needs to be its own function and have its own unit test. Sometimes a function that does just one thing and one thing well has more than one step to do that thing, and those steps don't necessarily belong in their own functions. Testability and composability are important, but that has to be balanced against locality of reference and context. When I see a colleague write in a code review that pieces of a function should be factored out of a larger function "just in case they'll be reused" I step on it hard. This is related to "over generalization" but not exactly the same.
- jayvanguard 10y agoGood point, the code locality is very important for maintainability down the road. When a single logical unit of functionality is unnecessarily split up across a file or even worse split into multiple classes it makes it much harder to understand.
- GrinningFool 10y agoI think a balance is possible. I tend to prefer to decompose functions for readability. A series of simple sequential function calls that describe what they do is more quickly readable than the same instructions listed as a series of compound operations. These function definitions are also typically defined in call order in the same file. If you need to know what the individual functions do you can scroll down the necessary few lines , but IMO as long as you name your functions honestly that isn't generally required to be ablr to understand what both what it's doing and where you're likely to need to make your change/fix at a glance. This style does require a lot of trust that everyone is following the same strategy - as soon as you have to doubt that accuracy of a named function in this approach, the benefit is lost. So it becomes important to keep them up to date - changing what function does must also change its name.
- Roboprog 10y agoThank you. I find layered stuff much easier to understand than massive clutter with "housekeeping" and "domain logic" stirred together, as well. My IDE allows me to leave the "documentation" panel open attached to a side of the window. As the cursor/caret touches an identifier, the documentation for that function/method or constant/variable appears automagically. But of course, I type reasonably well and write Javadoc/JSDoc on my stuff, so there is something to see. Others seem to rely on gargantuan "self documenting" (bullshit!) identifier names for the IDE to auto-complete, rather than actual documentation. Pet peeve.
- sidlls 10y agoSure, and it's great when a balance is struck. Too often, at least in my experience, the smaller chunks get shuffled out of the same file and into a nightmarish Rube Goldberg concoction in "utils" modules, packages and whatnot. I use primarily compiled C-like languages (C, C++ and Rust, specifically). I tend to put chunks of code that might be candidates for factoring out in their own scope block with a comment. Then, if the function really does become "too big" (for the very subjective measure that is), bits can be factored out on an as-needed basis.
- deepaksurti 10y agoThis resonates with my latest experience. I am working on a 3D game simulation and I have two player entities (Batter/Pitcher) who run on the ground. The running is done between to predefined control points/nodes in the scene graph which in turn is converted into a path. Every frame update I get the current location and move to the next location. // func buildPath(pathName): // builds a path between start and end control nodes fetched from the scene graph // func updateFrame(timeElapsed): // gets current location and move to next location using timeElapsed Now both these functions are not too big, but not too small either. However a reviewer mentioned I should split and I refused because they don one thing and do it succinctly. This useless dumb fascination with test practices and being rigid about it, causes grief, both for the code and the coder!!!
- TeMPOraL 10y agoAs GP wrote, > that has to be balanced against locality of reference and context. I'm also against splitting stuff like crazy, even though I tend to write small functions. For me, the primary concern is "how easy is this function to understand"? Sometimes splitting out code into separate functions actually hinders readability - especially if it makes reader jump around, or uses state passed implicitly between many small functions. I find good functions to be structured like good explanations - the function body should give you a general and complete understanding of what it does, and low-level details get outsourced to helper functions, that you can recursively review if needed.
- kbenson 10y agoI think part of the problem comes from the term "split". Some may assume you are a referring to splitting a function into two or more functions that are called from the original call point, while others may assume what you are referring to here, the separate of concerns inside the function into small sub-functions (which can also sometimes make them reusable) to encapsulate concern. I think the former is of limited use if any, while the latter is an invaluable tool to managing complexity of a code base. That people may assume the former when the latter is meant is unfortunate.
- 10y ago
- 20years 10y agoYes, yes! Having to locate and dig through 10+ different smaller functions that are called within another function is much harder to troubleshoot and work with. Especially when 95% of those are not re-used anywhere else. I know it may not be the popular belief but I would much rather work with a larger well written 50 line function that does what it is supposed to do vs having to navigate around a bunch of smaller 5 line functions that are never re-used anywhere else.
- collyw 10y agoMe too.
- Roboprog 10y agoWe need to not work on the same project :-)
- potatoyogurt 10y agoIf it's done right, though, you shouldn't have to navigate around all the smaller 5 line functions, at least not unless you're debugging. Each of the submethods should clearly describe what it does, and do that without any unexpected side-effects or implicit state changes. Effectively, what this does is raise the level of abstraction of the code, so that your 50-line function becomes a 10-line function, and you only need to read those 10 lines to understand what's going on. Sometimes when you're debugging, you might need to dip into these subfunctions, but when you do, you should be able to think through them as mostly independent units that just need to do what their name says they will do. This ideally limits the problem of having to load everything that your code is doing into your head all at once in order to reason through it. When you're just reading through the code normally, you should be able to gloss over the details of how the submethods work, just as you probably wouldn't feel compelled to dig into the internals of how a library call like Collections.sort works. The problem is that method decomposition is not always done well, and when there is unexpected behavior or implicit state changes, it can make bad code even harder to navigate. I'd also rather read a well-written 50 line function than a poorly decomposed cluster of 10-line functions. With proper care, though, method decomposition is super helpful.
- rdsubhas 10y agoCoincidentally covered, but just not in terms of functions :) 5.1. Sandwich Layers Lets take a concise, closely bound action and split it into 10 or 20 sandwiched layers, where none of the individual layers make any sense without the whole. Because we want to apply the concept of “Testable code”, or “Single Responsibility Principle”, or something. But you have expressed this way better than I did, thanks :)
- gravypod 10y agoFor me a functions is meant to represent an "idea", a way to "transform" your given into something you want. If you look at it this way it is easy to see when a function needs to be sub-divided. When an idea relies on another idea, it should be broken out. Otherwise, in the case that the lines are part of the main "idea" then it should remain. When grading homework for the course I TA for at my college it is evident that this idea is not understood. I have to admit, I don't actually understand it. It's just something I've taken to doing. I can't describe what I mean really and I don't think I've ever seen a good explanation. I think it's obvious that you practice the same way I do, but I still see a difficulty in articulating these thoughts. It goes without saying that, basically, we know that some things need to be moved out, and some things don't, but it's impossible to tell or describe to someone when these actions are taken. Does anyone know of some place that describes this well?
- matchagaucho 10y agoAs a long-time OOP enterprise practitioner, I'm forced to concede in recent years that functional programming (FP) had it right. Many challenges cited in this article become relatively simpler to navigate when thinking in FP terms (rather than OOP).
- sidlls 10y agoWell, I've never really used FP outside of hobby/toying around programming so I couldn't say. I'd say it applies to any programming paradigm, including OOP or non-OOP procedural programming.
- jghn 10y agoThe problem that always ends up with this is what defines "one task". I've rarely come across people who didn't agree with the notion that things should only do a single task, but I often find disagreement on the granularity of what a task is. FWIW I don't prefer small functions for reusability but for reasonability. It's a lot easier for me to discern what that block of 10-15 lines of code is doing if it has a descriptive name and a type signature.
- sidlls 10y agoWell, when those small functions are a series of other functions it can hinder "reasonability" just as easily as help it. There's no one right way to do it, really. My objection is mainly toward a zealous adherence to some subjective principle here.
- jghn 10y agoHonestly it's one of those YMMV things, as you say there is no right answer. That's why I said "for me" :) People are different and drill into information differently as well. Most of the arguments I've had over the years on this subject were cases where we were both right. To them X was more reasonable and to me Y was, and the two were mutually exclusive. As I write this it makes me wonder if this whole topic is a fool's errand, are we doomed to forever be trying To solve an unsolvable problem?
- sidlls 10y agoCouldn't agree more. It does seem like one of those many things where the team itself should probably determine a happy middle ground and stick to it.
- z3t4 10y agoThe problem with splitting function up a la OOP best practices will ironically result in zero code reuse because all functions will depend on each other.
- jaimebuelta 10y agoUnit tests are a fantastic tool, but they have clear limitations. Tests (unit, integration, system, acceptance or any other kind) should do two things: - while developing, help checking that the code is doing what it should (e.g. fixing the bug) - after developed, ensuring that the code keeps doing it. (ideally also ensure that it doesn't do anything not supposed to do, but that's much more difficult) That's why the best tests should aim for weak spots and try to give a good assurance that, if there are defects, the test should have a good chance of failure.... Those weak spots sometimes will be in single functions, but other times they won't. Creating tests just for the sake of having them, not adding a better chance of them failing when a problem arises is just wasted time. Or worse, coupling the tests with implementation details that are irrelevant (for example, this function is divided in two or three because is big, just to add readability) is just pointless....