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

161–170 of 220 posts

Re: Avoid Indirection in Code

#161

Post by kerkeslager »

Ma8ee wrote: post #70
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…

I very much disagree with you. "starts_with_foo" is a horrible name, because it only says how it is implemented, but doesn't convey any meaning or the intention of the function. When I read the code, I have to paus and think about why we do this. When I see a name like "is_foolike" I should have good enough understanding of the code that I know why we want to know if something is "foolike", and at that point, I shoul…

> "starts_with_foo" is a horrible name, because it only says how it is implemented, but doesn't convey any meaning or the intention of the function.

Two questions for you:

1. If `starts_with_foo` only communicates how it's implemented, what about `starts_with_foo(x)` communicates to you that it's implemented as `x.startswith('foo')` rather than `x[:3] == 'foo'`, or some other implementation?

2. If the intention of the function isn't to test whether x starts with 'foo', how is this not a bug?

Re: Avoid Indirection in Code

#162

Post by syn0byte »

neilwilson wrote: post #145

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

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!

Re: Avoid Indirection in Code

#163

Post by ubercode5 »

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.

Re: Avoid Indirection in Code

#164

Post by Ma8ee »

kerkeslager wrote: post #161
Ma8ee wrote: post #70

Earlier quoted context omitted.

I very much disagree with you. "starts_with_foo" is a horrible name, because it only says how it is implemented, but doesn't convey any meaning or the intention of the function. When I read the code, I have to paus and think about why we do this. When I see a name like "is_foolike" I should have good enough understanding of the code that I know why we want to know if something is "foolike", and at that point, I shoul…

> "starts_with_foo" is a horrible name, because it only says how it is implemented, but doesn't convey any meaning or the intention of the function. Two questions for you: 1. If `starts_with_foo` only communicates how it's implemented, what about `starts_with_foo(x)` communicates to you that it's implemented as `x.startswith('foo')` rather than `x[:3] == 'foo'`, or some other implementation? 2. If the intention of th…

Yes, it only says that we are checking that the first three characters are 'foo'. Exactly which commands you are using is irrelevant. You are telling us what we are doing on a too low level. With your logic anything that doesn't spell out exactly which assembly language commands that are used won't be implementation details

What is a bug? If the name of the function is IsFoolike, the definition of the function will be the authorative place in the code that tells us what "isFoolike" means. Then it is a semantic question if that is a bug or not. What do we mean when we say that something is Foolike?

Re: Avoid Indirection in Code

#165

Post by usrusr »

flukus wrote: post #18
rocky1138 wrote: post #10

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

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

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.

Re: Avoid Indirection in Code

#166

Post by carapace »

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

Re: Avoid Indirection in Code

#167

Post by JamesBarney »

lmm wrote: post #125
JamesBarney wrote: post #74

Earlier quoted context omitted.

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…

> The issue is fundamentally abstractions leak. Only people who've never used a good type system think this. Your function example demonstrates this pretty nicely: use a proper interval type for the two intervals and a proper return type rather than int, and then the answers to all your questions become obvious.

So say you replace it with a DateRange class so you end up with

range1 = new DateRange(start1, end1);

range2 = new DateRange(start2, end2);

DateRange overlapRange = GetOverlap(range1, range2);

You still don't know what happens when start1 is greater than end1?

What happens when they have no overlap? What does overlap return if it doesn't return null?

What happens if start1 and end1 are in local vs UTC time?

What happens if range1 and range 2 are in local vs UTC time?

When you get overlapRange.Duration.Days is this days in 24 hr increments or days by date?

And this is a really simple example. Things like ORMs are very leaky abstractions which make this mcuh worse.

Re: Avoid Indirection in Code

#168

Post by ncmncm »

he0001 wrote: post #49
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.

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.

Re: Avoid Indirection in Code

#169

Post by geodel »

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.

Re: Avoid Indirection in Code

#170

Post by tawy12345 »

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.

Post reply on HN