19 ms·

Avoid Indirection in Code

• keyle 7y ago
  That's a pet peeve of mine; I see it all the time when I work with lesser experienced developers, only I didn't know how to call it. I call it onion skin development, where the developer keeps hiding stuff in more layers of the onion, making my eyes water as I have to dig deeper and deeper to essentially find `a.foo(b)` under 12 layers of abstraction. They're so focussed on making everything look so purrty, they forgot that it's about telling the computer to do something, as clearly as possible.
  • seanmcdirmid 7y ago
    The onion is useful when thinking through a problem while writing code (add an abstraction layer as you try to eliminate pieces of complexity in layers), it usually doesn’t appear for aesthetic reasons. As understanding solidifies, the layers should probably be eliminated, and all that complexity should be smashed into one layer...or maybe not.
  • KayEss 7y ago
    > they forgot that it's about telling the computer to do something, as clearly as possible Or alternatively, it's to explain to the next reader of the code, as clearly as possible, how the problem was solved (in such a way that it can also be executed by the computer).
    • keyle 7y ago
      Yes I meant "as clearly as possible" <for the next guy>.
  • tsumnia 7y ago
    While I can agree with the sentiment, when is indirection okay vs. when does it become spaghetti code? Given [1], there needs to be a happy medium between 12 layers and all in a single method. I think with less experienced developers, the issue is diving too deep because they think they need to prove they aren't novices. However, its still a learning period for them where they just need a good mentor to tell them they can pull back a little. Outright avoiding indirection (I don't believe) does that. [1] http://www.cs.kent.edu/~jmaletic/Prog-Comp/Papers/fix-1993.pdf http://www.cs.kent.edu/~jmaletic/Prog-Comp/Papers/fix-1993.p...
    • kenhwang 7y ago
      My rule of thumb is 3 times. Whatever is abstracted away must be used at least 3 times. It's amazing how much preemptive prettification by abstraction it prevents.
      • Double_a_92 7y ago
        Abstraction is not mainly about avoiding repetition though. It's about (well...) creating abstraction. Imagine if you have to check permission to do something, and that check is really complex and long code. If would be better to have code like: if (hasPermissionToEdit(myUser)){ ... Than a ton of ANDs and ORs in that if, that nobody would quickly recognize as a permission check... even if it's just there once. As a minimum I'd put that in a properly named variable first, if I really wanted to avoid an extra function.
      • psychoslave 7y ago
        Keeping a function at a given cyclomatic complexity generally also give a good indication whether a function should be refactored. That's not completely the point of the article, but both issue are interesting metric for deciding the level of factorization. Also one need more accurate specification when defining a call deepness alarm. Most direct call to any random API will probably already explode a limit of 3, which doesn't necessarily say that the framework is brokenly atomized.
    • keyle 7y ago
      Agreed, I don't preach writing code like a caveman either!
    • ElonMuskrat 7y ago
      During review and debugging I need to understand what the code is actually doing. I would prefer not to chase through a linked-list of function to uncloak something as trivial as string comparison. There seems to be this insidious notion that engineers never need to understand implementation details, and therefore should aspire to bury program semantics through layers of abstraction that [often poorly] express the author's intent. This is a recipe to write systems that are insecure, and operationally un-maintainable.
  • ubertaco 7y ago
    Indirection is a tool, and there are both good reasons and bad reasons to use it (as with most tools). A good reason would be to clarify the why or the what by "glossing over" the how: # Unclear: list_range = range(0, len(movies)) for i in list_range: j = randint(list_range[0], list_range[-1]) movies[i], movies[j] = movies[j], movies[i] # Clear: movies = shuffle(movies) Yes, you're "hiding" the actual steps (the "how"), but in doing so, you're elevating your intent (the "why"/"what") to the forefront, and so you're making it clear what your code is supposed to be doing. This makes it easier to understand overall, and makes it easier to determine when your implementation and your expectations don't match (because your expectations are clearly described). A bad reason for indirection is one that's super-common in Java: namely, "we might need to make this swappable at some undetermined point in the future". It's the kind of thing that leads to public interface UserLookupService { // ... } public class UserLookupServiceImpl implements UserLookupService { // ... } That's just repetition for reasons that are at best forced by technical limitations, and at worst by paranoid decision-making. A mark of skill at coding (in particular, as a part of software development) is the ability to clearly convey intent to the reader. Sometimes, that means bringing a new concept into code that makes the problem easier to understand and talk about (to borrow an example from elsewhere in the comments here, introducing a DateInterval type can make common operations on two dates clearer). Sometimes, that means recognizing when the details get in the way of the point. We often do the same things when we write prose for humans to read. When writing prose for humans, it may help to introduce new clarifying terms like "time complexity" and "memory usage" so that we're not constantly explaining that when we say "performance" this time we mean "performance specifically in terms of how much RAM is used" and that time we mean "performance in terms of how much time is required to for the function to complete". It certainly often helps to edit down overly-detailed explanations that distract from the main point (possibly putting the fuller explanation into a footnote); if someone asks where you've been, how helpful is it to tell someone that you opened your garage door, got into your car, started your car, shifted gears into "drive" (or 1st gear), drove down your driveway, turned left....etc, as opposed to telling them that you went to the store to buy milk? The point I'm getting at here is that telling someone "avoid indirection" is a bit like telling someone "avoid summarizing." Sometimes, a full account is needed, sure -- like if you're on a witness stand -- but most of the time, summary is one of many useful tools to convey information clearly. Likewise, sometimes it's necessary to see every single explicit step being performed -- when doing in-depth performance optimization, for example -- but most of the time, indirection (especially by functionalization) is one of many useful tools to convey information clearly. Indeed, indirection in code is actually "safer" than summarization in writing. In code, you can always "go to definition", where in writing you may not have that option.
• keithnz 7y ago
  I don't think the author made a very good case for this. The case made on readability. But indirection is very common that if you can't read a piece of code without having to drill into the implementation of every function, lifes going to be quite painful. Depending on your language and editor / IDE, seeing implementation is often trivial. The only case, I see, for inlinig is where the pieces of code are very cohesive and tied together and will only be used as a single unit. Maybe that's what the author was trying to get at, don't break atomic units of code up.
  • mberning 7y ago
    I agree. The author actually makes a decent case for using this pattern though. I think it comes down to the skill and clarity of thought of the developer. If this pattern is used judiciously and the methods are named well then it adds very little cognitive overhead. Also, it is very refreshing to read about and discuss actual development.
    • keithnz 7y ago
      For sure, the pattern is good, having atomic units of code fragmented into pieces is not nice. This is essentially part of the Single Responsibility Principle, if things are together to suppport a single responsibility, don't fragment them. The trick is knowing "what IS the responsibility of this thing?" and not over doing it.
• mgerdts 7y ago
  Reminds me of my first day of python performance analysis, which led to this fix. https://web.archive.org/web/20130227000541/http://defect.opensolaris.org/bz/show_bug.cgi?id=11002 https://web.archive.org/web/20130227000541/http://defect.ope... https://github.com/oracle/solaris-ips/commit/53bb14f750dd1111bf8938278c2436e4141798dd https://github.com/oracle/solaris-ips/commit/53bb14f750dd111...
• dingoegret 7y ago
  Abstraction isn't the same as indirection. A facade layer isn't an abstraction. A double dispatch isn't an abstraction. An abstraction is a layer minus a detail that can be expressed in several different ways by components in that layer. The components act as translators that map their expression of the detail to the detail below them. Abstraction has a very specific implementation, usually polymorphism and a very specific intent. It's not just a synonym for nested function calls or helper functions or one stop shop facade or whatever.
• rocky1138 7y ago
  This could easily be remedied by our editors offering a keystroke to inline the function definition in a smaller font and different colour so that we may read it, in order, as though it were one big file. Once the gist of the function has been understood, another keypress could take it away. Visual Studio has this and it's called "Peek at function".
  • flukus 7y ago
    If we need more complicated tooling then the code is obviously less readable x.startswith version. Wrapping simple functionality like this inside "self documenting" functions is the bane of my existence when doing maintenance. Not only is the code less clear when you have to step into or peek through a myriad of micro functions but when you need to change them you have to analyse every code path to make sure the change is supposed to affect them all and that quickly becomes an exponential problem. So much of this is cargo culting attempts to keep methods short without understanding why long methods are a problem in the first place. That problem is the amount of state you have to juggle mentally, short abstractions like this that don't eliminate that state, so they create no benefit and the cost of obfuscating the code.
    • usrusr 7y ago
      Of course in real life maintenance coding, your journey will not be finished with a nice .startsWith("foo"), instead you encounter .startsWith(FOO) and can still count yourself lucky if you find FOO defined as "bar" (because $reasons) in a way that does not allow principle of maximum surprise redefinitions somewhere else. All for getting a somewhat reliable understanding of what the trivial entry point line does when trying to understand code downstream in the control flow.
• james_s_tayler 7y ago
  TL;DR - don't pointlessly wrap functions from the standard library in methods with non-standard names. Yes. Please. Stop. Doing. That.
  • justasitsounds 7y ago
    In this case the desired functionality can be encompassed in a single standard library method. However, the author of the code may have started with a much more complex implementation but still feels like `is_foolike()` is better at describing the _intention_ of the method than the eventual implementation. Replacing the descriptive method name with the actual implementation may mean the author now feels that they have to append a non-functional comment to explain that in order to test for foo-ness one only needs to do a simple string-prefix match.
    • james_s_tayler 7y ago
      You know how many times I've seen that exact scenario in code bases? Plenty. Some developers just do really pointless shit and then name is terribly to add insult to injury.
      • DonHopkins 7y ago
        Especially if your name is Ike and you don't appreciate being called a fool.
• sudhirj 7y ago
  The argument in the post is a bit of a strawman - for really simple examples like that indirection makes no sense. Saying "is_something_like" as an alias for "starts_with" is an unnecessary alias that does nothing for the readability or duplication of the code. Indirection really isn't necessary in this case, even if you use "starts_with" in a thousand places. It might make sense if you're trying to do a comparator, like "is_email_equivalent" - in that case the indirection would make sense - you might be doing a just a lower case check today, but in the future you might expand that to take into account Gmail specific equivalence rules. For stuff like "read_json_from_file", indirection makes a lot of sense. No point doing the whole "create_buffer", "read_file_to_buffer", "parse_json" deal every single time.
  • didericis 7y ago
    I’d hesitate to call the good abstractions you describe indirection. The term is most appropriate when describing opaque abstractions that force you into their inner workings in order to understand their purpose. In the original example, what exactly it means to be “foo like” is not obvious. You need to bounce into the other file/method in order to understand it’s purpose. In good abstractions, like the ones you describe, you can read the method and understand it’s intended purpose as is. In fact, a good abstraction will be easier to understand than inlining all it’s details. This is a semantic point, but I don’t think there really is such a thing as good indirection. Indirection is the result of bad abstractions. The author is basically just saying “don’t make bad abstractions”.
    • sbov 7y ago
      It doesn't matter how good your abstraction is, if someone needs to know the details it is at that point indirection. And there will always be some cases where someone needs to know the details. The better your abstraction, the less often. But someone, sometime, will still need to look at them.
      • didericis 7y ago
        True, there will always be people who need to know the details. That applies all the way down to CPU opcodes. I’m making more of a semantic point than anything else. If you define indirection as anything which causes you to jump from the code directly in front of you at any point, then yes, you can call even good abstractions indirection. I’d argue that such a definition makes the word “indirection” must less useful, as it becomes pretty much synonymous with “abstraction”. I think it’s better to reserve the word “indirection” to describe abstractions that require you to look elsewhere to understand what the abstraction actually is.
• gautam1168 7y ago
  i regularly see functions that are over 300 lines of code that make no other function calls except ajax. That shit is hard to read.
• kerkeslager 7y ago
  Okay, maybe it's just a poor example, but in the example used, the problem is definitely not too much indirection. If I was doing a code review and came to this: def is_foolike(x): return x.startswith("foo") I would comment, but my comment would be, "Could you name this function `starts_with_foo`?" This addresses both concerns mentioned in the article: 1. During review, when a reviewer is asked to verify that code is sensible before it can be merged into the main project. That reviewer probably has about a tenth as much time to spend as the original author does on that code. But if the reviewer comes to the code if starts_with_foo(x): do_something_with(x) They aren't likely to be misled in any way by the indirection. They can just keep reading, without having to look into the implementation of `starts_with_foo`, because either that name is accurate and they know what it does, or they'll discover that the name isn't accurate when they review the code. 2. While debugging future issues. This code will eventually be involved in a bug and some completely different developer will have to glance at this code to figure out what’s going on. They’ll have to understand some small section this code within a few minutes to determine what is relevant. They won’t be able to invest the time to understand the full thought process behind it, and a web of function definitions can slow down this process considerably. But again when they read the code, a good name means they can understand what the code does without having to read the implementation. I do think there's an argument to be made that indirection is a problem here, but it's a small problem compared to the enormous problem that `is_foolike` is a really bad name for that function. General rules for when to NOT pull something out into a function: 1. If it the function call wouldn't be clearer than the code (even a better name like `starts_with_foo(x)` loses some information contained in `x.startswith('foo')`, and the latter is readable enough that there's no real upside to the former. If the latter were even two lines long, it would become a lot more worth it. 2. If there's no repetition. Two similar pieces of code aren't enough: you don't understand from two use cases what pattern you're abstracting, so the result is just going to be a leaky abstraction. Three repeated pieces of code seems to be the sweet spot: now you have enough examples to know what's actually repeated, what should be arguments to the function, etc.[1] [1] https://blog.codinghorror.com/rule-of-three/ https://blog.codinghorror.com/rule-of-three/
  • justasitsounds 7y ago
    `is_foolike` is a bad name if you name your methods to only describe how they are implemented internally. At a high-level, in your business domain, you generally write your code to hide the implementation details and to describe the intention of the method EG. `is_carbonated` if is_carbonated(beverage): beverage.jiggle(false) vs if beverage.startswith("co2"): beverage.jiggle(false) (edit) for formatting
• a_t48 7y ago
  I really disagree with just the toy example. If it's just in one place, fine, but when you need to change the example to also do something else like check the end of the string `return x.startswith("foo") && !x.endswith("bar");` this style will end up biting you. Yeah, your IDE has tools that can help, but you might not catch all examples, especially in other branches.
  • sesteel 7y ago
    Yes! I understand that it feels inconvenient to read, but what is REALLY inconvenient is hunting down all occurrences of a magic string comparison that is poorly defined.
  • addicted 7y ago
    I disagree with the toy example even if it’s just 1 use (although to be honest, you could construct a different example where I would choose to abstract only if there were multiple uses). The big problem I have with the toy example is that both pieces of code convey different ideas to a human reader, and limiting myself to the situations the author finds the indirection cumbersome, such as code reviews, or debugging, both would lead the reader in different directions. The suggested solution would make a reader think the business requirement of that code is to check if the input starts with foo, whereas the indirection code suggests to the reader that the input should be like foo (which in this case happens to be by testing it starts with foo, but that may be incorrect, or may change). I think the author’s example really hurts their case. A better example may be where the intention indeed is to check whether the input starts with the string foo, but you want to trim the sample first. I’ve seen code that would abstract that out with a function trimmedStartsWith, or something, instead of just input.trim().startsWith(foo), where I think the former only makes sense if you’re doing it a bunch of time.
• sbov 7y ago
  It really depends. If you repeat this a lot, you might have a domain concept. If you can come up with a name that makes sense for your domain then you should probably take that domain concept and realize it within your code in the form of some kind of abstraction - e.g. a function call. Even if it is as simple as a .startswith call. If you can't come up with a name, kinda depends upon how prevalent it is. If you just do it a handful of times, no big deal.
• wawhal 7y ago
  I slightly disagree because I prefer the code to be consistent all across my codebase. Sure it is some effort to backtrack to the util functions, but I think the effort is worth it because I would rather have a developer spend multiple hours debugging and figuring out the code than having something that is a nightmare to iterate upon.
• xxxpupugo 7y ago
  Highly recommended this book: A Philosophy of Software Engineering: https://www.amazon.com/Philosophy-Software-Design-John-Ousterhout/dp/1732102201 https://www.amazon.com/Philosophy-Software-Design-John-Ouste... This is mentioned as one of the symptoms/appearance of bad code: Shallow/Passthrough methods, which results deep call chains, adding up cognitive overload. The author in this book recommends deep module over shallow module, which tends to end up with more cohesive code.
• ojbyrne 7y ago
  Decomposition and abstraction are fundamental tools of software design, obviously it can be done badly, as can anything. Carried to the logical extreme, this suggests that hiding the complexity of arithmetic operations like + and - is a bad idea.
• ncmncm 7y ago
  The simple rule is: every line of code has to earn its keep. Sometimes we really do need a one-line function to implement a common interface. Sometimes we need it so that, in the two places it's called, both get the new behavior when it changes. But abstraction for abstraction's sake is just stalling. The technical term is "ratiocination". It's a filthy habit.
  • he0001 7y ago
    I’m not so sure about this. A one liner may have its merit when you write it at that very precise location right now, but as the code grows the one liner may not have the same merit any more. But that may not always clear when you wrote it.
    • ncmncm 7y ago
      When any part of the program stops earning its keep, delete it. Feel proud of every deleted line. Deleting code adds as much value as writing code, but costs much less.
• djohnston 7y ago
  if the tooling is terrible maybe, but as long as i can click thru to see the referenced method in an IDE it's going to be fine
• winkelwagen 7y ago
  There is one thing that I’m missing, yes when the only thing you do is wrapping the standard library with a function it’s overkill. But when there is a business decision logic behind it I would still advice to extract it. Creating a function with a name tells you something about the why! I think peek definition could help you out when you are really dealing with it in the code base. I just wish it would be smarter. Peeking a function with 5 lines, peek it. If it’s larger, jump to it.
  • nomel 7y ago
    I do this when I know a future feature or requirement will make that function complex. I also will add a comment saying so, “putting this her since we’ll need x when y”.
• gitgud 7y ago
  It's not indirection, it's bad abstraction that's the real issue here. Consider the example used in the article: if x.startswith("foo"): do_something_with(x) if is_foolike(x): do_something_with(x) The problem with both of these variations is that the "if-statement" doesn't have any meaning behind it. There's no gain in the indirection presented here. Whereas the following code has meaning: if checkHasPermissions(x): do_something_with(x) The reader of the function is now able to understand the purpose of the indirection and the function has clear responsibilities. Remember although code is interpreted by machines it's read by humans...
  • baddox 7y ago
    I agree with you. I think it becomes pretty clear when applied to the example in the article of code review. If the abstractions are designed and named well, why should the reviewer need to jump to the declaration of the function in the if statement? Assuming the checkHasPermissions function wasn’t added or modified in the PR, the reviewer should just consider whether it makes sense to do_something if and only if checkHasPermissions.
    • barrkel 7y ago
      You'd think so, but abstraction names are like comments: they are seldom updated when the implementation grows more hairs. In older code, or code worked on by a larger team, I always chase into functions to understand the code, and don't rely on names. I'm also less inclined to indirect code unless the abstraction has genuine meaning, rather than being a compression technique for repetition, no matter how well named.
      • cottsak 7y ago
        Then the problem here seems like the lack of drive towards intention-revealing naming. It's burdensome, but worth it when the intention is revealed and prevents the navigation to the implementation. But like the author said, the reuse fallacy prevents this from having a good RoI until there is a significant usage of said abstraction.
    • 7y ago
• Rapzid 7y ago
  Pretty much everything written in Ruby before "Practical Object-Oriented Design In Ruby" by Sandi.. Haha, #KiddingNotKidding. Looking for a non-contrived and/or non-scarecrow example? Anything in the Chef code-base circa 2014 or prior.. No clue if it has changed. Ruby code bases were the poster children of DRY-gone-wild.
• rajangdavis 7y ago
  I feel as though this is a huge part of working with Rails as there is an incredible amount of magic that happens under the hood. From my limited experience, I think abstractions like these are helpful as long as there is consistency. This is easy to do by yourself but not always on a team; however, I think Rails has a good balance of conventions that allow indirection to be successful as long as the people writing the code use the same patterns and communicate effectively.
• gregmac 7y ago
  First of all, one of the biggest reasons to do this (and not mentioned) is unit testing. If there's several variations of input, it can simply testing because it's easier to write tests for smaller units of code (fewer or no mocks, and fewer parameters). This alone is probably enough to override any negative, in my opinion. Secondly, and in contrast to the point of the article, it can usually help readability. If you're wrapping standard library functions, then I agree, the examples in the article are bad candidates. If you're dealing with something where magic values are involved, for example, then something like the following can greatly help readability: /* Checks if a given user id was generated by the legacy system. These are either 5 digits numeric, or start with "B" or "L". */ is_legacy_user(userid) This method body can include the details about the strange legacy naming or links to documentation, and the code consuming this can focus on the logical implications without caring about these details. Put together, the code flow is easier to follow, and if you're in doubt about the method you can check out the unit tests to see what situations exist that might not be obvious at first glance. Edit: actually, I take that back. Even if you're just wrapping a standard library call, then there's really no issue so long as there's a domain reason (eg, is_legacy_userid() vs is_userid_fivedigits()), and there's unit tests. The unit tests can prove the standard library function is all that's needed.
  • pytester 7y ago
    >First of all, one of the biggest reasons to do this (and not mentioned) is unit testing. This is one way in which unit testing can cripple code. It's especially bad when code that mainly has the job of integrating different systems is unit tested - it typically means adding two additional, unnecessary layers of indirection to test some completely trivial piece of logic while the thing that is most likely to actually fail (the integration itself) goes untested. Those unit tests covering trivial logic still fail when the APIs are changed, but they almost never fail in the presence of an actual bug and present a huge maintenance cost for no tangible benefit. In some particularly integration heavy code bases I've seen literally every unit test is an expensive waste that spaghettified the code. Indirection is a good idea if you're creating a thin abstraction over a piece of complex logic, but that shouldn't be something that is done just to accommodate an irrational desire to see unit test coverage % go up.
  • 7y ago
• DubiousPusher 7y ago
  There's a lot of reasons to avoid indirection but I don't think decreasing the number of files you have to navigate is one of the better ones. I mean, who reads code linearly anyway. My hand hovers over F12 (the shortcut for go to definition in Visual Studio) half the day. Any good IDE will help with this. I feel like this person is like, I don't want to drill all these holes to build my deck. I'll use nails. And it's like, "dude, somebody invented the 3 inch self tapping deck screw; go to town."
  • dnautics 7y ago
    I think also this idea of navigating files is wrong; shouldn't these functions, generally speaking, be in the same file?
• hyperpallium 7y ago
  How should we decide whether to abstract something into a function? Abstract aspects likely to change, via interfaces that aren't, says Parnas (On the Criteria To Be Used in Decomposing Systems into Modules, https://www.win.tue.nl/~wstomv/edu/2ip30/references/criteria_for_modularization.pdf https://www.win.tue.nl/~wstomv/edu/2ip30/references/criteria...) abstract-unless-leaky: hide information you don't need. In practice, writing tests helps me see what is naturally part of a module vs. a usage of it. You want the module to contain everything it needs to do its job, and nothing else. But this assumes the modularization (its "job").
• satyenr 7y ago
  I don’t quite get the point of article. It essentially boils down to: “Abstractions — or rather, indirections — are bad for for readability, but they are good in some cases.” Umm... this can be said for pretty much anything. Over use of anything is bad. Inappropriate use of anything is bad. > Mostly, I want authors to be aware that there is a human cost to indirection that is felt more acutely by everyone reading the code except the original author. Sure. It’s a well known fact that abstractions have associated costs — nothing new there. However, the author seems to forget that not abstracting things can be equally costly — even when human readability is concerned. Imagine someone trying to figure our the business logic be reading code. All they need to know is some conditions are being met for certain steps to be executed. The actual steps involved in the check are irrelevant to the business logic. The condition can be modelled separately from the business logic and can even change independently. For example: def is_foo_condition_satisfied((self): return self.variable.startswith(‘foo’) In this case, it just happens that the satisfaction of a condition is implemented as prefix check. In the future, it might be changed to checking a flag or reading a database, or anything really. If you sprinkle the startswith() check all over you code, it will be hard to make the change. And it actually hurts readability. While reading a large codebase, you want to focus on specific parts of it — getting into the details of everything all at once does not make it easier. The most important thing to note is — programming is not confined to s strict set of rules. It’s not an exact science. You can’t define a strict a rules that must be followed. What makes sense somewhere is completely useless elsewhere. The Linux Kernel is full of goto statements, despite the fact that goto is considered harmful. Dogma Driven Programming must be avoided. Even well known programming practices are guidelines at best. One must evaluate whether a given rule fits a given situation. Coming back to the example at hand, functions are meant to divide the program by the logical tasks being performed, not by number of lines or any other metric. As I said before, it’s more art than exact science. Is the check for foolikeness of something a logically distinct task? It depends on the context. There is no way to write a cardinal rule either way. PS. I sincerely hope no one comes to me for code review with a monolithic function because of all the inlining — citing this article as inspiration. Edit: Typo correction.
• quasi 7y ago
  The point made by the article (avoid using due to problems for debugging and code review etc.) does not hold much water. 'Indirection' can be invaluable while separating 'how you got it' from 'what to do with it' (it=some data), for example. 'what you do with it' can remain unaffected by changing 'how you got it'. This will not mess with code review practices but make understanding the pieces easier. It will also make debugging easier as you can test both those pieces independently. What the OP calls indirection is just bad code or meaningless abstraction. Zed Shaw puts it well: "Abstraction and indirection are very different yet cooperating concepts with two completely different purposes in software development. Abstraction is used to reduce complexity. Indirection is used to reduce coupling or dependence."
  • satyenr 7y ago
    Yup, seems like the author had trouble following badly written abstraction and/or indirection and came up with the idea that they are bad. Abstractions are invaluable if you are dealing with a large codebase. Even the original author can’t keep everything in their head. Indirections are invaluable if you want to be able to modify code in the future without making a gigantic mess — and potentially breaking compatibility in the process.
    • hinkley 7y ago
      Did you know you can die from drinking too much water? Take care pursuing things for their own sake and not for a reason (I suspect you do but impressionable people are reading along with us).
      • satyenr 7y ago
        If you read my top level comment on this thread, you’ll notice I am saying exactly that — over use of anything is bad — as is inappropriate use.
    • jrochkind1 7y ago
      Well, good abstractions/indirections are hard. The wrong abstraction can be worse than no abstraction at all. And it can be non-obvious if you've got the "wrong" abstraction until much later when it's been in use for a while and had to be maintained -- and sometimes even then, we aren't good at recognizing that our pain is coming from the use of the wrong abstractions. We often are, without necessarily realizing it, reluctant to refactor existing abstractions once there, preferring to alter them slightly or even more add new ones on top (which can make things worse in the long-run). And this isn't necessarily irrational, refactoring/removing existing abstractions is expensive, and with no guarantee you'll get it "right" this time either. Sandi Metz says: > prefer duplication over the wrong abstraction https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstracti... Mechanically extracting things as in OP can often lead to "wrong" abstractions, which is what I get as the takeaway from the OP, which is a good reminder. You are right that abstraction is the main tool we use as computer programmers, one way or another. Thinking in abstractions is the main thing we do that characterizes computer programming in contrast to other endeavors. So we certainly can't give it up entirely. But this fact, I think, from my observations over my career, often leads us to over-abstraction ("over-engineering"), and the wrong abstractions. Because we are so taken with abstraction (it is neat, and you probably think so if you enjoy computer programming). It is good to be reminded that there is such a thing as too much as well as too little abstraction. Sometimes it's the "wrong" abstraction, but sometimes the "right" abstraction and the time it would take to arrive at it were unnecessary, and less/no abstraction would have served you just fine.
• cpeterso 7y ago
  See also Zed Shaw's "Indirection Is Not Abstraction". The blog post is a bit overlong, but Shaw's point is important: Abstraction and indirection are very different yet cooperating concepts with two completely different purposes in software development. Abstraction is used to reduce complexity. Indirection is used to reduce coupling or dependence. https://zedshaw.com/archive/indirection-is-not-abstraction/ https://zedshaw.com/archive/indirection-is-not-abstraction/
• tobr 7y ago
  It’s not just a problem for review and debugging. I think it’s even worse for refactoring and new feature development, because it makes it so much more difficult to know where to make a change. If the condition in the (silly) example needs to change, should you write a new function to replace the call to is_foolike, should you rewrite its implementation, or should you add a parameter that alters its behavior?
• UglyToad 7y ago
  While the example in the article isn't great for making the point I actually agree with the main idea. I understand the arguments for 'Uncle Bobifying' code but I think, as the article says, there's a balance to be struck. It's highly likely the next time I see your code (or my code if I'm coming back to it a month or two later) is when I need to fix a problem with it. While it's useful to have it split up into logically discrete chunks with sensible naming these 2-4 line functions far increase the mental load of the code. Given I'm debugging the code because there is presumably a problem with it I can't trust these sensibly named functions to do what they say, I need to check their internals to reason about changes in application state. Though well named functions at least help the maintenance programmer reason about the why they're equally liable to code rot as the comments they replace. As in all things there's a balance to be struck but I'd rather see a 20-30 line method than jump between 5 files because someone has read Clean Code recently.
  • quasi 7y ago
    Isnt this just 'good coding practices'? How is 'Avoid Indirection in Code' justified?
    • UglyToad 7y ago
      Yes, but 'good coding practices' is relative/subject to interpretation. I see this as a necessary counterpoint to the 'Clean Code to extremes' in a lot of other blog posts. It simply calls for some consideration before moving every little thing into another method. Granted the title is clickbaity but that's just the curse of the modern web.
  • guitarbill 7y ago
    Agreed, although I think the example isn't too bad. I often see such "helper methods" - usually in a class called "utils" or "misc" - when (novice) programmers "try to plan ahead": how should I write this so it can be re-used? But KISS is remains a good principle. Predicting the future is hard. If in doubt, don't put in the extra indirection/abstraction. If/when the next person needs that functionality, they can refactor it - and the simpler/more straightforward the code, the easier it is to refactor! When the logic is already split over multiple methods, refactoring in a different direction becomes harder. Often, it's enough to verify there's only one caller, and then stuff it back into one method before refactoring. Still, more annoying than if they hadn't of overthought it.
• cryptica 7y ago
  I agree with OP, if a function just returns the result of another function without any additional processing then it usually means that one (or both) function's name does not accurately describe what it does. The name of a function should abstract away from "how it does something" not "what it does".
• de_watcher 7y ago
  "All problems in computer science can be solved by another level of indirection"
• blibli 7y ago
  Maybe we should avoid reading code.
• DarkWiiPlayer 7y ago
  My mental benchmark for the cost/payoff of indirection is usually > Would this code be easier to read if A) I inlined the code or B) I replaced it with a descriptive function name. Function could be read as Class or Method if you're dealing with OOP. One important criterion for this is that other programmers already know the language, not your program. Writing your own function can help to put a meaningful name on a set of behaviors, but it also means the introduction of a new specific set of behaviors the reader needs to keep in the back of their mind. It's kind of similar to Jakob's Law [1] in a way, but applied to source code. (In a way, source code readability in't all that different from UX, if you think about it) [1] https://lawsofux.com/jakobs-law https://lawsofux.com/jakobs-law
• 2rsf 7y ago
  Like everything else you need to consider the context. I was involved in a rather big internal automation project written in Python, it involved a few platforms and targets and was ran open source style where each team could hook up to the infrastructure independently or make changes to it using PR. Some of the code ran remotely using RPC, so part of the code sat in different repositories. Naturally this called for multiple levels of abstraction and indirection. The results ? a big ugly pile of levels if you are trying to understand what the code does, that led to bugs, unnecessary complexity since it's hard to foresee the future and always abstract the way the code will need and the worst was that people simply gave up on trying to contribute do to the complexity of the otherwise simple code.
• vast 7y ago
  The main point here is worth a broader discussion. Part of the problem is that we tend to hide behind mental concepts that a ambiguous, incomplete or just bad. DRY is a awful "thing" (a decree at best). In the worst interpretation it simply says "never write the same code twice". There is no balance or end to it. It doesn't have a competitor or alternative. It somewhat implies that it is always good. It doesn't define a scope where/when it should be applied. There is simply no broad agreement how to use it in practice. DRY touches multiple concepts, each too complex to put behind three tiny letters. OP fails to make a great point, but the direction is right. We cannot take DRY and "code reuse" laws as granted. Abstractions and indirections have their downsides. It increases systemic complexity and may add dependencies. It certainly limits how easy the full system can be understood by humans.
  • AstralStorm 7y ago
    The alternative to DRY is... Repeating yourself. Which is fine once or thrice but not too many times. Second alternative is code generation, which brings its own problems, but is relatively clean. (Custom build systems are a pain.) You have to keep balance to not turn the code into some DSL for example. Then you can also do macros, especially if the language has a decent enough support. (In C++ the equivalent is not C macros but templates and RAII pattern.) And finally, you do not need to move code out to necessarily reuse it all the time, or even make it public. Things that work together best stay together, and strengthen encapsulation.
  • tetha 7y ago
    The counterpoint to DRY doesn't seem to have a name, but it very much exists. I know it under the names of 'oversharing' or 'premature sharing'. Practically, this tends to extend the question of DRY: Is the code the same in two places, and will the code /change/ in the same way in these two places? Can we delay sharing this code to have more time to figure out if the code changes in the same way? Maybe currently two integrations with two external applications are just the usual socket/newline separated json at the moment, so you could share the implementation. But maybe one integration gets replaced with thrift, one gets replaced with protobuf and suddenly you end up with an abstraction that's full of <if service.protobuf...> and that'll be ugly.
• shtolcers 7y ago
  On the other hand fixing "is_foolike" will be pain to fix if used in many places. And - "This non-linear reading process requires more mental focus than reading linear code." - isnt't this a god thing? Good abstraction requires a bit more mental involvement and much less monkey-job when fixing the code.
• ivanhoe 7y ago
  Having a central place for logic is a HUGE pro, while both those cons apply to a very narrow class of situations: having a super simple condition logic on one side vs. not very smartly named indirection. In real life (at least mine) you'll often have a complex or cryptic-looking conditions (requiring that you're deeply familiar with business logic, laws, tax rules, etc.), and abstracting them into sensibly named functions actually helps the review process and debugging IMO.
• XeO3 7y ago
  Imho, this type of abstraction should be allowed in case of code repetition. So the reviewer/debugger will have to go non linear only one time. In any other case the whole code should be in one file.
• asavinov 7y ago
  Indirection [0] is a quite big topic and applying it can result in both significantly improving the code and getting more problems than benefits. Therefore, the question is when indirection is appropriate. One opinion [1] is that "We can solve any problem by introducing an extra level of indirection" Another opinion is expressed in this post (avoid indirection). In the context of this example, indirection is related to the problem of modularization and separation of concerns. In particular, a typical question is whether we want to hide some details or use the functionality directly. [0] https://en.wikipedia.org/wiki/Indirection https://en.wikipedia.org/wiki/Indirection [1] https://en.wikipedia.org/wiki/Fundamental_theorem_of_software_engineering https://en.wikipedia.org/wiki/Fundamental_theorem_of_softwar...
  • microtherion 7y ago
    I always liked the second part of this quote: "…except for the problem of too many levels of indirection"
• skywhopper 7y ago
  As with anything, the key here is to strike the right balance. Sometimes the code behind the ‘if’ test will be worth factoring out and sometimes it’s better to keep it simple. Impossible to say what the line is, as it depends on the team, the language, etc. But in relation to the actual problem of legibility to a new developer still building up the mental model of the code, this is an area where tooling can help. You ought to be able to view the called-method’s body with a keystroke, without losing the current context, in an IDE or a code review tool. I’m sure some IDEs do have such features. Yes this is harder in some languages than others but in general it’s too bad these sorts of features aren’t built in to Github and Gerrit as well.
• janpot 7y ago
  Ok, to take a real world example instead: if (url.startsWith('http://')) { vs. if (isAbsoluteUrl(url)) { If the next developer comes by in 2 months to fix the case for https:// https:// urls (and protocol relative ones in 6 months), they'll immediately be able to spot the intention of the code and can easily fix it in the abstraction layer that's already in place. Moreover, the fix will be applied every other place this faulty assumption about their input was made.
  • gmueckl 7y ago
    Your example works because it avoids duplication a magic string constant. If the string wasn't a constant, but another variable, thw whole argument changes.
    • janpot 7y ago
      My example is literally the real world version of the example in the article, only thing I did was change string values and identifier names. If you say my example works as an abstraction, it means that it successfully refutes the argument that the article tries to make.
  • GuB-42 7y ago
    url.startsWith('http://' http://') may be better, especially if used only once. For example, what is "isAbsoluteUrl()" supposed to do? Does it match anything that starts with "word:", maybe "word://" maybe it is limited to supported protocols, what about validity, encoding, etc... For example "mailto:foo@bar.com" is an absolute URL, and a correct one that will work in your browser, but if your intention is to download something, it will fail. In fact, just by looking at isAbsoluteUrl(url), I don't have any idea about when the condition will be true. Maybe you want something more explicit like isAbsoluteDownloadableUrl(url). But still, it doesn't tell you everything, in particular, it doesn't tell you what kind of checking it does. Is it a full validation or just a quick check? And sure, you can add documentation and all that but in the end, that's typically how you add in 50 lines of code for no good reason. url.startsWith('http://' http://') may not be generic but what it does is crystal clear. And maybe, when https support will be added there will be a bug. But if isAbsoluteUrl() is just "return url.startsWith('http://')" http://')", it is still be a bug. I am not saying that isAbsoluteUrl(url) is bad, just that the article also applies to your "real world" example. And as mentioned in the end of the article "functions are still a good idea ... there is some balance to find here".
• zimbatm 7y ago
  It's more of a problem with OOP language because the function call might hide a side-effect. So now the reader has to go and read the function definition to make sure no side-effect is happening in there. With functional languages the mutable data is available in the reader's context. The implementation might still be mysterious but hopefully the function name is good enough to give a clue about that.
  • mikekchar 7y ago
    I'm just being a bit grumpy, but there are non-pure languages that aren't OOP, and there is no reason why you can't write OO code that is where all objects are immutable. However, it is a good point that mutability is a major problem. It's just not correct that this has anything to do with OOP.
    • zimbatm 7y ago
      Fore sure. I was painting a large brush and you can find instances of mutability in functional languages and subsets of OOP that are immutable. Things like value objects, the builder pattern, dependency injection, ...
      • mikekchar 7y ago
        Possibly you aren't aware, but both FP and OOP are orthogonal to mutability. There are lots of mutable FP languages (for example, the first one ;-) ). These days it is popular to make pure FP languages, but that was not the case 40 years or more ago. Similarly, Alan Kay had quite a lot to say about mutability in Smalltalk. I'm not aware of any pure OO languages, but very few people knew how to deal with persistent data structures until Chris Okazaki's PhD thesis on the topic ~1996, if I remember correctly. Mutability is really up to the programmer. Even in C++ you can have const data structures and even const functions -- which are guaranteed to be pure. C++'s mistake was that it didn't make const the default; a mistake that Rust corrected. Back in the 90's all of my C++ code had const strewn all over it -- it was on practically every line. It was really clear to those of us doing it that you had to separate your "const" code from your "non-const" code because if you ever decided to omit that "const", it would rot your entire code base in a very short time frame -- mutability is really contagious. I know it's popular to hand-wave and say, "Well most OO programmers don't write immutable code and lots of design patterns depend on mutability so therefore OOP is defacto-mutable while FP is defacto immutable", but this really obfuscates the true nature of the issue. There are programmers who prefer to write mutable code (in any paradigm) and those who prefer to write immutable code. It's quite easy to write OO code in an immutable way and most statically typed OOP languages even give you tools to help you do it. The reality is that many programmers just don't care. Of course, those programmers never reach for a pure FP language, but there are still heaps of old school FP programmers who would never write immutable code (I know several personally). Perhaps you knew all that as well, but if so, I would ask a favour that you don't say "OOP is mutable" because it is highly misleading for people who don't understand the issue well (and frustrating for programmers like me who enjoy writing immutable OO code).
• Simon_says 7y ago
  This feels like somebody is testing the limits of what nonsense developers can be made to read.
• z3t4 7y ago
  Code is like a bunch of cords. Each time you add something, it's like adding another cord, and at some point you will have "spaghetti", and you pull out every cord, unravel it, connect everything neatly and use zip-ties to hold everything in place. The problem is when you use the wrong abstraction, eg. you start zip-tie:ing before even knowing what other cables to add. So don't be afraid of using the copy/paste function. Make it easy to unravel.
• reuben_scratton 7y ago
  John Carmack's thoughts on this topic are worth a read: http://number-none.com/blow/blog/programming/2014/09/26/carmack-on-inlined-code.html http://number-none.com/blow/blog/programming/2014/09/26/carm...
• unnouinceput 7y ago
  The connection has timed out The server at www.matthewrocklin.com is taking too long to respond. too many HN readers perhaps?
• k__ 7y ago
  When I started programming, I tried to get functions below one screen height, have one return and use mutations often. Today, I try to do big functions, multiple returns and const where I can. I still use code-blocks sometimes to scope variables. I sometimes end up with a few hundred lines of code per function, but I can usally scroll over it and find what I need without jumping around. Especially in JavaScript the addition of const and async/await helped much with this.
• Thunderya 7y ago
  This made me think: is there an IDE or plugin that makes it easy to visualize the source code of a function by inlining the sources of the undercalled functions? For example, before the transformation: function isFooLike(x) { return x.startsWith("foo"); } function isBarLike(x) { return x.startsWith("bar"); } function bigFunction(str) { if(isFooLike(str)) { console.log("Foo!"); } else if(isBarLike(str)) { console.log("Bar!"); } else { console.log("Unknown!"); } } And then: function bigFunction(str) { if((function(x) { return x.startsWith("foo") })(str)) { console.log("Foo!"); } else if((function(x) { return x.startsWith("bar") })(str)) { console.log("Bar!"); } else { console.log("Unknown!"); } }
  • ktpsns 7y ago
    While I find your inlined example very bad to read, I think there are many attemps IDEs do for this. I remember some showing the function stub of a called function when moving the mouse on the call instruction. I also know multiple editors that allow to quickly browse the source code with CTRL+Enter (or CTRL+Mouse click) in a similar fashion as we click on links in the web. I personally never found indirection to be a problem. It's rather the solution.
    • Thunderya 7y ago
      Yes unfortunately my example is not very readable, but it was to illustrate the idea of including the sources of the called functions. Indeed, many IDEs allow you to easily display the source code of one function at a time, by navigating to the sources of the function or by displaying it in a pop-up, but the idea I had in mind is to be able to view the source code of the function with all its dependencies at a glance.
• rb808 7y ago
  Both are clearly wrong, should have been using proper IoC with a fooCheckerFactory creating a checking object that inherits from a generic interface that is passed in the context.
• taneq 7y ago
  One very important reason to avoid this kind of unnecessary indirection is that it wrecks locality of reference for anyone reading it. They end up spending all of their mental energy on skipping around the codebase trying to follow the thread of execution, making it very hard to assemble a coherent picture of what the code's actually trying to do.
  • vbsteven 7y ago
    I like IntelliJ's "Quick Definition" shortcut for this. It allows me to see the definition of a function in a popup window so I can quickly glance at what a certain function does without the full context switch.
    • taneq 7y ago
      I've only spent a few weeks using IntelliJ but from that time it seemed like a really well constructed dev environment. Definitely gave me the feeling that this is what modern software development should be like.
      • vbsteven 7y ago
        I believe it is. The big thing that made IntelliJ completely click for me is when you buy the Ultimate edition and you download some plugins for Ruby,Node,Python or whatever languages you use you have a full IDE that works for pretty much any language. In my eyes it is a platform for doing modern software development.
• mannykannot 7y ago
  In theory, this abstraction of small details is not a problem, but in cases like this, I would prefer to see the inlined version, simply because, in many languages, a function call raises the possibility that it has side-effects that you might want to know about. In code that is abstracted to the max, that is sometimes a difficult question to answer, even if your IDE lets you easily trace the call stack as you read. There is a related problem that causes more trouble, but it cannot really be blamed on the initial abstraction. It goes like this: you have something, such as a foo-ness test, that is needed in several places, so you abstract it as a function for all the right reasons. Later on, one of those uses changes, and needs slightly different logic than the other cases. Instead of writing a new function (that may, as part of its implementation, call the original), the person making this change (not you, of course!) modifies the original function to handle both cases. This, by itself, makes the code more complex than necessary, and it gets worse if the criterion for which path is taken in the function is not something that has any significance for someone who arrived at this function through reading the code for one of the other cases that call it. And if the new path has a side effect, now all the other cases are also calling a function that potentially has side effects... This is how code rots.
• nickthemagicman 7y ago
  Could it be lack of good tooling? I dont understand why there isn't a code editor that will inline functions for you?
• lriuui0x0 7y ago
  I agree with the conclusion largely, but I think the reasons listed in the article are not good. Personally I feel the problems of doing too much abstraction are belows. 1. It takes a lot of time. 2. You might end up with bad abstration that in the end hinders you. You then have to work around the abstration you created. 3. More functions create more entries you need to understand in your mind, this can sometimes make code harder to reason about.
• neilwilson 7y ago
  This is back to the usual intractable problem - naming things. If you name the function properly and it abstracts the operation of the function precisely then you have added information, not taken it away. You understand what 'has_kettle_boiled' means. Inlining functions makes it difficult in a sequence to work out what are the separate steps. Splitting those out into a function - particularly if they take the temporary variables with them - makes it easier to understand what the higher level function does, not harder. Perhaps we should revisit stepwise refinement as a technique and the need for functions to be loosely coupled and highly cohesive.
  • syn0byte 7y ago
    What was the kettle boiling? How long ago did it start boiling? Is it still boiling? That Sea level boil or high altitude boil? ARRGH! your function name isn't descriptive enough!
• husainalshehhi 7y ago
  Well, indirection can be quite helpful even for simple checks: if is_json(somestring) { // do something } function is_json(string s) { return JsonLib.convert(s) != NULL } this indirection is easier to read rather than thinking too much about what JsonLib.convert(s) != NULL mean. This does not hinder debugging because you only debug this is_json function when the code does not evaluate is_json as expected. Every other time, you debug what's in the if statement. The given example x.startswith("foo") is simple enough to include it directly. However, if there is a business reason for why "foo" is checked here, it will be difficult to understand why the check is there. This will be more difficult to read a code like this: if x.startwith("-") do something instead of if is_yaml_node(x) do something def is_yaml_node(x) x.startwith("-")
• admax88q 7y ago
  Yeah avoid indirection. Instead of > x.start_with("foo") do > x.length >= 3 && x[0] == "f" && x[1] == "o" && x[2] == "o" Now the reviewer doesn't have to jump to the definition of "starts_with." Isn't that way more readable?
• rileymat2 7y ago
  >However, there is also a cost to this behavior. When a new reader encounters this code, they need to jump between many function definitions in many files. This non-linear reading process requires more mental focus than reading linear code. I think this is the problem right here. As a new reader you should first trust the function name does what it says it does and continue linearly. Later on, if you need to go back and look at the details or where the function name misleads or does more than it says.
• roland35 7y ago
  There are a lot of examples of heavyweight indirection in Hardware Abstraction Layers (HAL) on microcontroller vendor code - ST is one of the worst offenders! In their sensor code library I counted 9 layers deep of functions to simply send a command to a magnetometer over I2C. I find this code hard to work with since without actually debugging the code there is no easy way to just click on a function and see what is called - a lot of functions reference a data structure of I2C function pointers...
• ubercode5 7y ago
  In general I disagree. In large codebases you're going to have a number of abstractions that you'll have to deal with by their nature. Flattening these can cause a scenario where you have a long function with clusters of lines of code organized in a block are doing different functionality that isn't clearly stated. This may include a comment at the start of each block discussing what the next few lines are doing. And without abstraction, the functionality later may not be cleanly decoupled. Debugging these types of functions also adds significant mental load trying to see the forest through the trees. From the article it seems like the bigger issue might be debugging style. For me, when debugging those abstracted functions should be treated and trusted as black boxes until you have sufficient reason that function is the issue, and then dig into it and ignore the rest of the function. Trying to build a mental model by effectively flattening every function takes huge cognitive load, and only is feasible for small amounts of code. Once the codebase is large enough it's going to become impossible to do this.
• carapace 7y ago
  Err, this isn't bad advice, just kinda junior-level. First, if you're defining interface indirection to support encapsulation and you wind up never rewriting (or adding another) implementation behind that "external function" or method, you probably didn't need it. (YAGNI, yo?) Second, if you're defining interface indirection and it makes it harder to understand the code, you're doing it wrong anyway. - - - - Indirection that hurts is like when you do dispatch through a dict of functions and now all your tools, debugger, etc. can't tell you about the static structure of your code: dispatch = { 'name': handler_func, ... } ... dispatch[selector](*foo, **bar) That kind of thing seems like a good idea but: > "Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it?" –Brian Kernighan, "The Elements of Programming Style", 2nd ed., chapter 2 (Also, notice that, in Python at least, derp.baf(foo, *bar) is the same thing, eh? EH??)
  • keymone 7y ago
    well technically, if you type-annotate the dict you should be fine: dispatch: Mapping[str, Callable[arg_types, ret_type]] = {}
    • carapace 7y ago
      That's neat. I hadn't realized type hints had gotten that good now. Cheers! I've had issues reading code where I'm trying to trace control flow and I hit some dispatch point in the code and it's suddenly really hard to tell where to go from there. I do (did) a lot of Python programming and sometimes it's a little too dynamic, ya know what I mean? :-)
      • keymone 7y ago
        type hints got good, editors and typecheckers didn't really :) personally i like these dispatches more than giant if/switch statements for no reason other than better separation and management of logic. as for debugging - stepping through code in ipdb is still the way to go.
• geodel 7y ago
  I don't know much about Python programing. In Java though I deal with ludicrous amount of indirection and 'design pattern' turdlets which deeply hurt code comprehension everyday. Just last week I was handed a project with single functionality: 'Doing ldap bind on receiving userid/password over http and return success/failure'. This project has about 40 Java files in 18 directories. As far as enterprise projects go it follows all best practices of Java and Micro services etc. But I find this project absolute turd and hopeless to refactor. A clean rewrite would be only sane thing.
• tawy12345 7y ago
  I think some commenters don't understand the challenges of reviewing code contributions as an open source maintainer. Many people already feel like they're doing you a favour by contributing any code. This is amplified if they've followed "best practices" and broken it down into many tiny functions and objects. As a maintainer, having to review and maintain code that is abstracted in the wrong way is a major headache. Trying to tell someone that their code is over-abstracted risks starting a debate on the code review and generally derailing the process. An article like this at least sets some precedent and gives you something to point to.
  • asdfman123 7y ago
    For my reference, can you give me an example of properly abstracted code and overly abstracted code?
• vendiddy 7y ago
  I wonder if the real solution to the indirection is better tooling. If I can't easily see the contents of is_foolike, should I stop writing my code like this or should the editor be showing me these details more easily? The real problem: it takes me too much time to see what's inside the function.
• JackFr 7y ago
  "Avoid Indirection in Code" is a foolish thing to advocate, and the example provided is more confounding than explanatory. Abstraction is the key. Abstraction can be used to reduce incidental complexity by hiding an implementation. Abstraction can be used to reduce the cognitive load of the essential complexity by making the code look like the problem domain. But there is a cost to abstraction. A person reading your code may not have the same mental model of the problem, and might’ve be working at a higher or lower level of abstraction, in which case the "indirection" might be a stumbling block. Weigh these costs carefully.
• choiway 7y ago
  This is an overly simplistic complaint that leads to the argument that you should never use a 3rd party library.
• bcheung 7y ago
  There's nothing wrong with this kind of "indirection", I agree there are times when you might not want to do this but I would put it in terms of cognitive overload. By substituting a series of lower level concepts/tasks/procedures into a single keyword the reader can more easily think about the problem domain. That's a good thing. If they know the abstraction, it reduces the number of things the reader must keep in their head. However, if the reader is not familiar with the abstraction they will need to spend time to familiarize themselves. This can break down when trying to understand an abstraction leads to the "endless rabbit hole" problem. Basically, you have to open another file to see how that abstraction works, then it uses some other abstraction which you need to open another file to read, and so on. If the depth is too great the reader will lose context and won't be able to fit the problem in their head. To avoid this, pull more of the functionality into the function instead of outsourcing it to an abstraction. Favor writing code that reads sequentially rather than as a series of jumps through a bunch of files. As always, this is largely a matter of taste so use your best judgement and balance abstraction with pragmatism.
• raiflip 7y ago
  I wonder how many people who decry abstraction and indirection would choose to work directly in Assembly rather than in higher level languages.
• doomjunky 7y ago
  But all problems in computer science can be solved by another level of indirection. ~David Wheeler
• CodiePetersen 7y ago
  If you have defined the function and it is used in many places then you only need to read it once and remember everytime you come across it. How is recoding it every time going to help the readability. I think you just need to make sure the functions purpose is clear either.