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

81–90 of 220 posts

Re: Avoid Indirection in Code

#81

Post by falsedan »

baddox wrote: post #51
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):…

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

> 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?

How will the reviewer know if the abstraction is well-designed or named without reading it? Code quality attitudes change over time so sometimes the standard project-provided helpers end up not matching the intuitions of more recent contributors.

Re: Avoid Indirection in Code

#82

Post by silveroriole »

kerkeslager wrote: post #53
justasitsounds wrote: post #47

Earlier quoted context omitted.

I'm sorry that this will come across as rude, but I feel like you're repeating something you've heard but didn't quite understand. You're right, it does come across as both rude and condescending, and you know it, so don't apologise. In my previous post I hinted that the name of the function should "describe what the function does", which is the same as "describe the intention of the function", because if the functio…

> `is_foolike` to me, implies `test_for_abstract_quality_foo`. Yes, which is why when `is_foolike` tests that a string starts with 'foo', that's rather unexpected. If `is_foolike` actually describes the intention of the function, then the function testing for the string beginning with 'foo' is a bug, because it doesn't do what it's intended to do. Referring to "the quality of starting with 'foo'" as "abstract quality…

Ok, I’m curious. Let’s say that I want to fall into that if statement if my product is named something like “foo”, costs under $100, isn’t discontinued, and has sold over 100 units in the last month. Fine, you can say now I have overcomplicated the domain and I should go back to requirements, but these complicated things do happen. Surely it’s acceptable to name my function “isFooLike” rather than “startsWithFooAndCostsLessThan...” etc, especially if I suspect management may change some of those figures later?

If that’s acceptable, why is an IsFooLike which only checks one condition unacceptable, even though it (imo) expresses the same intention?

Re: Avoid Indirection in Code

#83

Post by AstralStorm »

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 the deepest level possible.

Re: Avoid Indirection in Code

#84

Post by AstralStorm »

vast wrote: post #80

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

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.

Re: Avoid Indirection in Code

#85

Post by chii »

quasi wrote: post #56

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

Too many people create abstractions without also creating the 'means of combination' for that abstraction.

If the means of combination is not created, the abstraction will end up being big and complicated, and non-extensible.

Re: Avoid Indirection in Code

#86

Post by quasi »

hinkley wrote: post #65
quasi wrote: post #56

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

While delegation involved indirection, indirection does not necessarily mean delegation. As other people have voiced, there are concerns about following the plot. I think it’s a bit like writing. I can paint a clear picture in your head or I can torture you slowly while relating the same four basic facts. While I have achieved the task in both you may not wish to work with me long if I only ever achieve the latter.

Agree on the first point, which is why the "for example". Regarding second: standard practices, clear thought, good abstractions, readable documentation are all necessary and have much much more impact on the lack of torture. The OP (and I think you) are saying that indirection can lead to torture and hence avoid using it (the title of the article). Misuse or lack of understanding of any technique, principle et.al will lead us and others to confusion and torture. IMHO there is nothing inherently wrong with using indirection, say, as long as we understand what we are doing (as with a lot of other things). And because, as you so rightly said, impressionable people are reading along with us, we don't want to leave the impression that 'indirection', say, is somehow a villain here.

Re: Avoid Indirection in Code

#87

Post by blub »

JamesBarney wrote: post #74
UglyToad wrote: post #61

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

The issue is fundamentally abstractions leak. And if the code is overly "Uncle Bobified" the abstractions will leak bugs. On the other hand if you under "Uncle Bobify" the code will be very difficult to read because you won't be able see the forest for the trees. This is one of the advantages of comments and local functions. You can inline a function an add a comment. With the comment providing the abstraction and th…

All of those questions can be answered through comments. I'd vote for a separate function.

Some of the answers can be encoded in the type system through the use of time units (like std::chrono), optional return values and design by contract (preconditions). Unit tests could be added to further clarify some edge cases.

Re: Avoid Indirection in Code

#88

Post by shtolcers »

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.

Re: Avoid Indirection in Code

#89

Post by ivanhoe »

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.

Re: Avoid Indirection in Code

#90

Post by benj111 »

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…

You're reading an awful lot into one 2 line throwaway example on a different topic.

CheckHasPermission() could be permission to update the score on space invaders.

Plus how are the other 2 examples in any way better from a security POV.

Post reply on HN