Avoid Indirection in Code

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

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.

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/

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.

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.