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

41–50 of 220 posts

Re: Avoid Indirection in Code

#41

Post by ju-st »

justasitsounds wrote: post #24
kerkeslager wrote: post #15

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…

`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

In your example `is_carbonated` is a private method. In an public interface you would want to hide the implementation details but surely not in a private method.

Re: Avoid Indirection in Code

#42

Post by Rapzid »

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.

Re: Avoid Indirection in Code

#43

Post by rajangdavis »

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.

Re: Avoid Indirection in Code

#44

Post by gregmac »

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.

Re: Avoid Indirection in Code

#45

Post by DubiousPusher »

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."

Re: Avoid Indirection in Code

#46

Post by nomel »

winkelwagen wrote: post #39

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. P…

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”.

Re: Avoid Indirection in Code

#47

Post by justasitsounds »

kerkeslager wrote: post #38
justasitsounds wrote: post #24

Earlier quoted context omitted.

`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

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. Yes, at pretty much ANY level, you should write your function names to hide the implementation details and to describe the intention of the function. 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 int…

  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 function doesn't do what it's intended to do, that's a bug.
`is_foolike` to me, implies `test_for_abstract_quality_foo`. `starts_with_foo` implies an assertion a string beginning with a particular prefix

  If we're representing beverages as strings which are somehow guaranteed to begin with "co2" if the beverage is carbonated, and we've decided that the deserialization should be mixed into your "high-level, in your business domain", the program is so badly written that we're not going to get any truths about good programming from it
I feel like you're intentionally misunderstanding my point. Please don't critique my entirely fictitious codebase as if it represents anything other than an abstract example. The point being that even though, in this (again) entirely fictitious example, the implementation is very simple, the intention of the method is different from it's implementation

Re: Avoid Indirection in Code

#48

Post by pytester »

gregmac wrote: post #44

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…

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

Re: Avoid Indirection in Code

#49

Post by he0001 »

ncmncm wrote: post #34

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.

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.

Re: Avoid Indirection in Code

#50

Post by hyperpallium »

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...)

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").

Post reply on HN