Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Avoid Indirection in Code

matthewrocklin.com

Post reply on HN
Print view View original

91–100 of 220 posts

Re: Avoid Indirection in Code

#91

Post by mav3rick »

AstralStorm wrote: post #83
gitgud wrote: post #40

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):…

Congratulations, this pattern is responsible for more security bugs than any other. Security should not be an "if" but must. Presumably down to instruction level or preferably to hardware that does TLB. This thing is a likely TOCTOU error. What happens if I pass the check and enter the function just not having the permissions? The only way this is acceptable is if the code is proven to never do that, preferably to th…

It was an example. It could very well have been checkForSomeNonCriticalThing(x).

Re: Avoid Indirection in Code

#92

Post by XeO3 »

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.

Re: Avoid Indirection in Code

#93

Post by ElonMuskrat »

tsumnia wrote: post #11
keyle wrote: post #2

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 forg…

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 the…

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.

Re: Avoid Indirection in Code

#94

Post by ElonMuskrat »

isolli wrote: post #73
gitgud wrote: post #40

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):…

Agreed. I remember it as: "show what the code does, not how it does it".

That works great when the semantics of the code match the written intent. This is fine for battle tested libraries and pieces of infrastructure.

However for code reviews and debugging an engineer must actually read and understand the 'how' to prove and disprove the 'what'.

Re: Avoid Indirection in Code

#95

Post by geocar »

gitgud wrote: post #40

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):…

In many cases speaking of the foolike nature of a value is not dissimilar to speaking to the primeness of an integral: Is a value prime? And now here's is an algorithm for determining whether this value is prime.

The motivation for many splitting out a prime-testing function is that primality testing is hard to do efficiently, not that there are many kinds of primes or many kinds of integrals. Indeed, in some languages "primes" is a builtin (p:), or a trivial derivation {(2=+⌿0=(⍳⍵)∘.|⍳⍵)/⍳⍵} for some domain, and that's the only domain our application is concerned with so is-prime is a (⊣∊primes) -- is foolike-ness like that?

Nothing is being abstracted here, and nothing is gained by indirection: Does this is-prime routine even deserve a name?

Sometimes the foolike nature has nothing to do with the value: Is an apple foolike? It isn't today, but it might be tomorrow! (That's numberwang!) Is this account locked (go check the disk!)? Sometimes we encode that the account is locked by putting an asterisk in the password field -- this won't match any hashes, so the login will always fail, and a "is_locked" routine might start exactly this way: def is_locked(x): return x.password.contains(42) but in the future we want to be sure that this isn't a _copy_ of a locked account or there's a synchronisation issue, so we go check and then def is_locked(x): return x.userid in locked_accounts

Nothing is being abstracted here either- we're still only talking about giving this algorithm a name (or maybe delaying the discussion about which algorithm gets this name). So why don't we just say 'something in x.can instead of x.can('something')? Because the former means set membership and the second may be able to not reify the set. Proxies like __in__ might help, but it's not just the syntax: Our language has let us down (clearly, not enough laziness! time to haskell all the things!), and so we cheat: The locked button sets a flag on the user object, instead of adding a user to the real list of locked users.

But what if our language were smart? Why wouldn't we write x.userid in locked_users?

> Whereas the following code has meaning:

And yet it is wrong! It has a nasty security bug in it that you will never find, and the next guy will never find. Fools and children would argue endlessly that something was wrong with the ticket or user story that led to this code, or demand that they didn't have enough experts on the team, but no matter how smart you are (or you think you are), you'll write a piece of code that is so short it cannot possibly be wrong and yet it is.

Code hiding (whether you call it indirection or abstraction) also hides bugs, and yet few of us have the time or attention to stare at the bits blurry with a steady hand and a magnetised needle, so there'll always be some amount of code hiding. The real trick (if there is one) is to do as little of it as is possible.

Re: Avoid Indirection in Code

#96

Post by asavinov »

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

[1] https://en.wikipedia.org/wiki/Fundamental_theorem_of_softwar...

Re: Avoid Indirection in Code

#97

Post by _urga »

catmanjan wrote: post #55
gitgud wrote: post #40

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):…

Even better (IMO), "hasPermissions" It isn't clear whether the "check" prefix means the function returns based on whether the checking was successful or not.

I'll do you one better...

  if authorized(x):
    do_something_with(x)

Re: Avoid Indirection in Code

#98

Post by skywhopper »

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.

Re: Avoid Indirection in Code

#99

Post by cakoose »

AstralStorm wrote: post #83
gitgud wrote: post #40

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):…

Congratulations, this pattern is responsible for more security bugs than any other. Security should not be an "if" but must. Presumably down to instruction level or preferably to hardware that does TLB. This thing is a likely TOCTOU error. What happens if I pass the check and enter the function just not having the permissions? The only way this is acceptable is if the code is proven to never do that, preferably to th…

Like others said, not the point of the example.

But even then, there's not enough context to determine if there's a TOCTOU bug. For example, what if it's using an RDBMS transaction?

Re: Avoid Indirection in Code

#100

Post by janpot »

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:// 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.
Post reply on HN