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 forgot that it's about telling the computer to do something, as clearly as possible.
The onion is useful when thinking through a problem while writing code (add an abstraction layer as you try to eliminate pieces of complexity in layers), it usually doesn’t appear for aesthetic reasons. As understanding solidifies, the layers should probably be eliminated, and all that complexity should be smashed into one layer...or maybe not.
> they forgot that it's about telling the computer to do something, as clearly as possible
Or alternatively, it's to explain to the next reader of the code, as clearly as possible, how the problem was solved (in such a way that it can also be executed by the computer).
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.
My rule of thumb is 3 times. Whatever is abstracted away must be used at least 3 times. It's amazing how much preemptive prettification by abstraction it prevents.
Keeping a function at a given cyclomatic complexity generally also give a good indication whether a function should be refactored.
That's not completely the point of the article, but both issue are interesting metric for deciding the level of factorization.
Also one need more accurate specification when defining a call deepness alarm. Most direct call to any random API will probably already explode a limit of 3, which doesn't necessarily say that the framework is brokenly atomized.
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.
Indirection is a tool, and there are both good reasons and bad reasons to use it (as with most tools).
A good reason would be to clarify the why or the what by "glossing over" the how:
# Unclear:
list_range = range(0, len(movies))
for i in list_range:
j = randint(list_range[0], list_range[-1])
movies[i], movies[j] = movies[j], movies[i]
# Clear:
movies = shuffle(movies)
Yes, you're "hiding" the actual steps (the "how"), but in doing so, you're elevating your intent (the "why"/"what") to the forefront, and so you're making it clear what your code is supposed to be doing. This makes it easier to understand overall, and makes it easier to determine when your implementation and your expectations don't match (because your expectations are clearly described).
A bad reason for indirection is one that's super-common in Java: namely, "we might need to make this swappable at some undetermined point in the future". It's the kind of thing that leads to
public interface UserLookupService {
// ...
}
public class UserLookupServiceImpl implements UserLookupService {
// ...
}
That's just repetition for reasons that are at best forced by technical limitations, and at worst by paranoid decision-making.
A mark of skill at coding (in particular, as a part of software development) is the ability to clearly convey intent to the reader. Sometimes, that means bringing a new concept into code that makes the problem easier to understand and talk about (to borrow an example from elsewhere in the comments here, introducing a DateInterval type can make common operations on two dates clearer). Sometimes, that means recognizing when the details get in the way of the point.
We often do the same things when we write prose for humans to read. When writing prose for humans, it may help to introduce new clarifying terms like "time complexity" and "memory usage" so that we're not constantly explaining that when we say "performance" this time we mean "performance specifically in terms of how much RAM is used" and that time we mean "performance in terms of how much time is required to for the function to complete". It certainly often helps to edit down overly-detailed explanations that distract from the main point (possibly putting the fuller explanation into a footnote); if someone asks where you've been, how helpful is it to tell someone that you opened your garage door, got into your car, started your car, shifted gears into "drive" (or 1st gear), drove down your driveway, turned left....etc, as opposed to telling them that you went to the store to buy milk?
The point I'm getting at here is that telling someone "avoid indirection" is a bit like telling someone "avoid summarizing." Sometimes, a full account is needed, sure -- like if you're on a witness stand -- but most of the time, summary is one of many useful tools to convey information clearly.
Likewise, sometimes it's necessary to see every single explicit step being performed -- when doing in-depth performance optimization, for example -- but most of the time, indirection (especially by functionalization) is one of many useful tools to convey information clearly.
Indeed, indirection in code is actually "safer" than summarization in writing. In code, you can always "go to definition", where in writing you may not have that option.
Honest question: isn't that kind of separating classes out into interfaces necessary for proper testing? When should you avoid splitting them out to interfaces and implementation classes?
I'm curious what you mean by "necessary for proper testing". I know a lot of people use that pattern (single-implementation interfaces) for use with a dependency-injection container, and then they mock the interfaces...but that doesn't buy you any more than mocking the implementation classes themselves would.
In terms of when to split them out, my rule of thumb is "when there's more than one implementation class that does the same function".
Good examples are things like Comparable/Comparator, Reader, Writer, Serializable -- these are all descriptions of behavior for which multiple implementations exist. Interfaces, in general, are adjectives (or at least adjective-ish).
Bad examples are things like CheckoutService, ShoppingCartManager, JwtTokenManager -- these are all descriptions of specific components within an application that have a single implementation. These are nouns, and generally they're a sort of "proper noun" (in that they refer to one single thing by its name). If you don't already have many implementations for the behavior they implement, then you don't need an interface (since an interface is just a way of describing a set of "external-facing" contracts which many specific implementations may fulfill -- like List vs LinkedList/ArrayList).
The reason why is simply that it's just waste. It's more code for more code's sake. At best, you're just clicking "go to definition" one extra time, and at worst your code is confusing (because someone might assume the presence of the interface means it's "swappable" behavior, rather than integral application logic).
The "YAGNI" ("You Aren't Going To Need It") principle applies here: if you don't know you're going to need it, don't build it. Once you do know that you're going to need it, then you build it. Anything before that point winds up being wasted time/effort.
I don't think the author made a very good case for this. The case made on readability. But indirection is very common that if you can't read a piece of code without having to drill into the implementation of every function, lifes going to be quite painful. Depending on your language and editor / IDE, seeing implementation is often trivial. The only case, I see, for inlinig is where the pieces of code are very cohesive and tied together and will only be used as a single unit. Maybe that's what the author was trying to get at, don't break atomic units of code up.
I agree. The author actually makes a decent case for using this pattern though. I think it comes down to the skill and clarity of thought of the developer. If this pattern is used judiciously and the methods are named well then it adds very little cognitive overhead.
Also, it is very refreshing to read about and discuss actual development.
For sure, the pattern is good, having atomic units of code fragmented into pieces is not nice. This is essentially part of the Single Responsibility Principle, if things are together to suppport a single responsibility, don't fragment them. The trick is knowing "what IS the responsibility of this thing?" and not over doing it.
Abstraction isn't the same as indirection. A facade layer isn't an abstraction. A double dispatch isn't an abstraction. An abstraction is a layer minus a detail that can be expressed in several different ways by components in that layer. The components act as translators that map their expression of the detail to the detail below them.
Abstraction has a very specific implementation, usually polymorphism and a very specific intent. It's not just a synonym for nested function calls or helper functions or one stop shop facade or whatever.
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 change is supposed to affect them all and that quickly becomes an exponential problem.
So much of this is cargo culting attempts to keep methods short without understanding why long methods are a problem in the first place. That problem is the amount of state you have to juggle mentally, short abstractions like this that don't eliminate that state, so they create no benefit and the cost of obfuscating the code.
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.
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.
You know how many times I've seen that exact scenario in code bases? Plenty. Some developers just do really pointless shit and then name is terribly to add insult to injury.
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.
I’d hesitate to call the good abstractions you describe indirection. The term is most appropriate when describing opaque abstractions that force you into their inner workings in order to understand their purpose.
In the original example, what exactly it means to be “foo like” is not obvious. You need to bounce into the other file/method in order to understand it’s purpose.
In good abstractions, like the ones you describe, you can read the method and understand it’s intended purpose as is. In fact, a good abstraction will be easier to understand than inlining all it’s details.
This is a semantic point, but I don’t think there really is such a thing as good indirection. Indirection is the result of bad abstractions. The author is basically just saying “don’t make bad abstractions”.
It doesn't matter how good your abstraction is, if someone needs to know the details it is at that point indirection. And there will always be some cases where someone needs to know the details. The better your abstraction, the less often. But someone, sometime, will still need to look at them.
True, there will always be people who need to know the details. That applies all the way down to CPU opcodes.
I’m making more of a semantic point than anything else. If you define indirection as anything which causes you to jump from the code directly in front of you at any point, then yes, you can call even good abstractions indirection.
I’d argue that such a definition makes the word “indirection” must less useful, as it becomes pretty much synonymous with “abstraction”. I think it’s better to reserve the word “indirection” to describe abstractions that require you to look elsewhere to understand what the abstraction actually is.
I think the idea here is that "foolike" is some kind of meaningful characteristic that happens to be testable by looking at a string prefix - not that the author is just making a helper to test string prefixes.
E.g.:
function isActiveCustomer(status) {
return status.startsWith("active")
}
is, I presume, the sort of thing the author means to suggest.
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]
`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)
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 intention of the function", because if the function doesn't do what it's intended to do, that's a bug.
The reason I say I don't think you understand what you're saying is that none of what you're actually saying applies to the examples you're giving.
`is_foolike` doesn't describe the intention of the method. It gives an entirely vague and inaccurate view of what the method does. So even though we both agree that functions should describe the intention of the method, you're saying `is_foolike` is an okay name even though it doesn't do what you say it should?
`starts_with_foo` describes the intention of the method. It doesn't describe the implementation: `starts_with_foo(x)` might expand to `x.startswith('foo')` or `x[:3] == 'foo'`, but I don't care which, because the name accurately describes what it does either way.
Your example doesn't elucidate. 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 our "high-level, in your business domain", the program is so badly tangled that we're not going to get any truths about good programming from it.
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
> `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 foo" isn't hiding implementation details, it's being opaque about what a function does.
Put another way, "what a function does" isn't its implementation. "How a function does what it does" is its implementation.
> `starts_with_foo` implies an assertion a string beginning with a particular prefix
Yes, exactly. Because that is what it is intended to do, we hope, since that's what it does. That's NOT the implementation: there are plenty of different ways to implement testing whether a string starts with 'foo', and the name `starts_with_foo` isn't coupled to any of them.
> The point being that even though, in this (again) entirely fictitious example, the intention of the method is different from it's implementation
Yes. `starts_with_foo` is also different from the implementation, while still describing the intention of the method.
Could you explain to me why you think `starts_with_foo(x)` describes the implementation `x.startswith('foo')` and not some other implementation (such as `x[:3] == 'foo'`)?
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?
> 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?
The idea with naming is to capture as much of the meaning of the function as possible, and 'isFooLike' doesn't really capture any meaning (what does it mean to be like a foo?). In business, there are typically names for the topics, like `is_profitable_transaction` or `meets_sales_targets`, which may indeed encompass some very complex logic, but are a coherent idea, so naming is usually a bit easier than this. But you're right, sometimes the requirements are bad and you don't have a chance to go back to requirements before a deadline. In those cases, I don't think the naming matters much because no name you come up with is going to represent the concept. So I guess `is_foolike` might be the best you could come up with, but it's certainly not good code. I'd also be less likely to pull out a function in the first place because it's a premature abstraction: if the abstraction isn't coherent enough to be named well, we probably don't understand it well enough yet to abstract it out.
> If that’s acceptable, why is an IsFooLike which only checks one condition unacceptable, even though it (imo) expresses the same intention?
It's not the same intention.
In the more complex case, you have a bad name because the name doesn't describe the (overly complex) intention of the code, but there isn't a better one.
In the second case, you have something that does a clear thing, so you should name it what it does. For a more complex case, you're not going to be able to capture every nitpicky detail of what the function does in the name, so you have to describe it in a broader concept. But with a short simple function like this, there really isn't an excuse for using a vague description.
I think you're still ignoring my explanation that what a function does and intends to do is different from how it does it. So I'm going to again insist that the two of you answer these questions to prove they actually understand my points before disagreeing with me:
1. I'm claiming that in correctly-working code, the intention of the code IS what it does. Is there a case where code would do something other than it's intended to do, and this isn't a bug or at least misleading code?
2. I'm claiming that what a function does is not the same as its implementation. Why are you claiming `starts_with_foo(x)` coupled to the implementation `x.startswith('foo')` and not `x[:3] == 'foo'`?
This is a really, really terrible way to name functions, or anything. I have seen code that tried to put the spec in the name, and it was awful.
A name that is long enough to tell you all you might need to know about what the thing named does is too long to be usable at all.
A name needs to be long enough to distinguish it from the other things being named. That's what naming means. If it also gives you a hint about which thing, of the things named, it really does, that makes it perfect.
Anything beyond that adds cognitive load, making it exponentially worse with each syllable added.
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.
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 should not have to worry about how it is implemented. It is only if I got reason to believe that there's something wrong with the function or when I review that particular function that I need to worry about the implementation details.
In addition, if we for some reason want to change what we mean with foo-like (say to "contains("foo"), we suddenly have to change all calls to the function, in addition to the defintion.
> "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?
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?
> 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
Nothing in `starts_with_foo` describes what commands are being used--it describes what it does, not how it does it. With your logic, telling what the function does at all is too low a level.
> 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.
Okay, so why not just name your functions, `aaaaa`, `aaaab`, `aaaac`, etc.? If the name of the function is `cdyfj`, then the definition of the function will be the authoritative place in the code that tells us what `cdyfj` means.
Aren't you worried that `isFooLike` gives us too much information about the implementation? After all, the string "foo" is in the implementation. What if you want to test for "bar" later? Then you'll have to change all the places where `isFooLike` is used, in addition to the definition!
> What is a bug?
1. You're claiming that the name should tell us what the intention of the function is.
2. You're claiming that the intention of the function isn't `starts_with_foo`.
3. The function tests to see that the string starts with 'foo'.
The only way all three things can be true is that the intention is if the function does something it's not intended to do. That's the definition of a bug.
> What do we mean when we say that something is Foolike?
The fact that nobody could possibly answer this question is exactly the problem I'm pointing out. You might as well just name the function `cdyfj`, because `isFoolike` doesn't actually give you much more information than `cdyfj` does.
> With your logic, telling what we are doing at all seems to be too low a level.
No, we need to say what we are doing on the right semantic level for that part of the program. It is rarely describing exactly what we are doing, because then only function of functions will be to abbreviate common patterns of code. The main function of functions is to abstract. At this particular level, Foolike has a semantic meaning to the reader of the program who knows what a Foo is and what isn't. And in this particular version of the program we happened to implement it from the first characters of the string. But maybe later we decide that it anything that inherits from a Foo, or has the Foo interface, or just walks, but not necessarily quacks, like a Foo.
And the reason I don't call the function some nonsense is because then it is hard for the readers of the code to understand the meaning of it. And it might not help even if the they read the definition of the function, because the definition isn't the meaning. It doesn't say anything about why it is interesting to know if the string starts with 'Foo' or not (which is, as I have mentioned, the problem with the name you suggested).
>> What do we mean when we say that something is Foolike?
>The fact that nobody could possibly answer this question is exactly the problem I'm pointing out.
No, we already know what foolike means. It is part of the domain that we are working with. The people who wrote the code and the people who read the code are assumed to have enough domain knowledge to know what a Foo is, and they understand me when I say things "we better use a Foolike here, otherwise we'll get a Bar problem".
Let's use a bit less abstract example. Say I implement the function NextInvoiceNumber(). My first implementation might be something like {return ++n} (because I'm making a proof of concept and I'm in a hurry). You seem to argue that a better name of the function would be something like AddOneToNAndReturnN(). But then the reader of the code won't understand what the purpose of the function is, because nothing says anything what n is good for and why we are interested in getting the next n. And the moment I start to add different number series for different articles and some of them increment with 10 and some with 100, the name AddOneToNAndReturnN() will be wrong. That will not be a problem for NextInvoiceNumber(), because that is perfectly normal for anyone who has done any work with invoicing. Invoicenumbers are part of the domain that the developer must know about.
> At this particular level, Foolike has a semantic meaning to the reader of the program who knows what a Foo is and what isn't.
It sure doesn't. Maybe readers are much smarter than me, but I don't know that it means to be foolike until I read the function definition. Apparently something is foolike if it is a string starts with 'foo', but I certainly wouldn't be able to guess that from the name. And I still don't know what a Foo is or isn't.
> But maybe later we decide that it anything that inherits from a Foo, or has the Foo interface, or just walks, but not necessarily quacks, like a Foo.
Okay, so you're naming this based on what it might do in the future rather than what it does? How are you predicting the future here? And when it does the future thing, will you change the name so that it is talking about the future again, since you have such an objection to names referring to what a function presently does?
> And the reason I don't call the function some nonsense is because then it is hard for the readers of the code to understand the meaning of it.
The concept of being like a food IS some random nonsense: it's hard for readers of the code to understand the meaning of it.
> Let's use a bit less abstract example. Say I implement the function NextInvoiceNumber(). My first implementation might be something like {return ++n} (because I'm making a proof of concept and I'm in a hurry). You seem to argue that a better name of the function would be something like AddOneToNAndReturnN(). But then the reader of the code won't understand what the purpose of the function is, because nothing says anything what n is good for and why we are interested in getting the next n. And the moment I start to add different number series for different articles and some of them increment with 10 and some with 100, the name AddOneToNAndReturnN() will be wrong. That will not be a problem for NextInvoiceNumber(), because that is perfectly normal for anyone who has done any work with invoicing. Invoicenumbers are part of the domain that the developer must know about.
Okay, I'm guessing you mean something more like:
int getNextInvoiceNumber() {
return ++this.invoiceNumberCounter;
}
And that's perfectly reasonable, because getNextInvoiceNumber describes what the function does.
A name like addOneToInvoiceNumberAndReturnInvoiceNumber would be bad, not because it's inaccurate, but because it doesn't tell you what the function does in context.
We don't have any similar context to concern ourselves with for 'starts_with_foo'.
It seems like going back to the beginning of the discussion, you guys assigned some meaning in your heads to the adjective "foolike" that you didn't choose to share with me and decided based on this meaning in your head that I was wrong. Perhaps the name `is_foolike` is a better name within the imaginary context you've created, but I was talking about the code that was linked on this thread, not the code you've imagined and didn't tell me about.
We where using what was obviously an abstract example. We could imagine that Foolike was some property that was meaningful for some domain that someone worked in. You do seem to have problems with abstractions in general. And in the end, when I realised what you needed to understand, even you understood.
I really disagree with just the toy example. If it's just in one place, fine, but when you need to change the example to also do something else like check the end of the string `return x.startswith("foo") && !x.endswith("bar");` this style will end up biting you. Yeah, your IDE has tools that can help, but you might not catch all examples, especially in other branches.
Yes! I understand that it feels inconvenient to read, but what is REALLY inconvenient is hunting down all occurrences of a magic string comparison that is poorly defined.
I disagree with the toy example even if it’s just 1 use (although to be honest, you could construct a different example where I would choose to abstract only if there were multiple uses).
The big problem I have with the toy example is that both pieces of code convey different ideas to a human reader, and limiting myself to the situations the author finds the indirection cumbersome, such as code reviews, or debugging, both would lead the reader in different directions. The suggested solution would make a reader think the business requirement of that code is to check if the input starts with foo, whereas the indirection code suggests to the reader that the input should be like foo (which in this case happens to be by testing it starts with foo, but that may be incorrect, or may change).
I think the author’s example really hurts their case. A better example may be where the intention indeed is to check whether the input starts with the string foo, but you want to trim the sample first.
I’ve seen code that would abstract that out with a function trimmedStartsWith, or something, instead of just input.trim().startsWith(foo), where I think the former only makes sense if you’re doing it a bunch of time.
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.
I slightly disagree because I prefer the code to be consistent all across my codebase.
Sure it is some effort to backtrack to the util functions, but I think the effort is worth it because I would rather have a developer spend multiple hours debugging and figuring out the code than having something that is a nightmare to iterate upon.
This is mentioned as one of the symptoms/appearance of bad code: Shallow/Passthrough methods, which results deep call chains, adding up cognitive overload. The author in this book recommends deep module over shallow module, which tends to end up with more cohesive code.
Decomposition and abstraction are fundamental tools of software design, obviously it can be done badly, as can anything. Carried to the logical extreme, this suggests that hiding the complexity of arithmetic operations like + and - is a bad idea.
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.
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. Peeking a function with 5 lines, peek it. If it’s larger, jump to it.
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”.
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):
do_something_with(x)
The reader of the function is now able to understand the purpose of the indirection and the function has clear responsibilities.
Remember although code is interpreted by machines it's read by humans...
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 checkHasPermissions.
You'd think so, but abstraction names are like comments: they are seldom updated when the implementation grows more hairs.
In older code, or code worked on by a larger team, I always chase into functions to understand the code, and don't rely on names. I'm also less inclined to indirect code unless the abstraction has genuine meaning, rather than being a compression technique for repetition, no matter how well named.
Then the problem here seems like the lack of drive towards intention-revealing naming. It's burdensome, but worth it when the intention is revealed and prevents the navigation to the implementation.
But like the author said, the reuse fallacy prevents this from having a good RoI until there is a significant usage of said abstraction.
> 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.
It’s a valid question, but at some point in a code review you have to trust an abstraction layer, lest you end up reading semiconductor physics papers for every PR.
Disagree. Someone attempting to do something while unauthorized is oftentimes not an exceptional state and regular business logic depending on how access is given to different endpoints. Do not use exceptions, which are "heavy" (capturing call-stack etcetera), for something that commonly occurs.
Put another way: exceptions are exceptions to the normal program flow. They should not be used to communicate expected errors. I think of them as analogous to the `if (do() != 0) { goto out; }` paradigm of C.
In my experience, this isn't always true. Exceptions are great in the context of web requests. If anything goes wrong, we have defined consistent behavior (re: http 404). It's so effective, there's a whole host of checked exception codes.
> Put another way: exceptions are exceptions to the normal program flow.
There's no iron law or mathematical proof that says this is optimal. It's just a preference some people have. Exceptions are the error handling mechanism for many programming languages. Authorization errors are a type of error. Under this construction, exceptions are the appropriate way to handle authorization errors, including when a principal attempts to access an resource they are not authorized to access.
Using an exception for authorization failures avoids TOCTOU security holes. Also, in many cases, there are intermediate layers of the call stack that don't have any useful contribution to failure handling; the best they can do is forward the failure return value up the stack. And if one part of an operation fails because of a lack of permissions, it is usually dangerous to continue processing without handling the failure, which is the default behavior that failure return values have in most languages.
By contrast, the performance cost of capturing stack traces when there's an authorization error is insignificant. How many authorization errors do you have per second in normal operation? For many systems, the answer is down in the millibecquerels to microbecquerels.
Under these circumstances, the balance leans quite heavily to the side of using exceptions to report authorization failures.
This is interesting and compelling. I wonder about spambots though, when authorizing a service exposed to the web. Doesn't it get expensive pretty quickly when a bot decides to scan the site and floods it with requests that all exception?
Actually, it would be a perfectly fine mini-encapsulation as-is. If you need to know whether something is foo-like or not, it's far better to have that test in one place rather than repeated all over the codebase. Otherwise debugging or modification would be a nightmare.
Even from a code U/X perspective this is better. How is someone going to know that x.starts_with("foo") means that the object is foo-like, rather than some subset comparison only? The name "is_foolike()" tells them unambiguously what information the function will return and more importantly, what the information means. Abstraction is about assigning higher order meanings to groups of information and behaviors so that we humans can keep the concepts straight in our minds, and is_foolike(x) is a lot easier to understand the underlying meaning of than x.starts_with("foo").
How you react to something being foo-like or not is a question for a higher abstraction level to decide, and present a unified front from there, like get_foolike_items(y), which in turn is needed by get_high_value_targets("sweden") (because all high value targets have foo-like metadata), which is in turn passed to launch_nuclear_missiles(targets).
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.
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.
All the same I think you may have thrown out the abstraction with the bath water.
> Does this is-prime routine even deserve a name?
Absolutely. If you saw it over and over you might start just reading it as "is-prime" instead of going through the process of mentally interpreting it each time. But what about similar-looking functions with similar functionality? You want a programmer to go and make you a promise that it has the meaning (if not the implementation) you're thinking about. That promise is the same as "code-hiding".
> But what about similar-looking functions with similar functionality?
Even though we're unlikely to need this construction more than once:
def is_prime(x): return x in primes(x)
This one (or one just like it) appears quite common:
def is_in_set(x,y): return x in y
But Guido already noticed that! That's why he made an "in" operator in the first place! As soon as our routines become too generic, we're just making APL without arrays, and that's why taste matters so very much.
> That promise is the same as "code-hiding".
If I can see the implementation at the same time as its usage, then the code is not hidden. Ipso facto.
But to expand more on what I mean by this, consider that close() is hiding code: You get all of the bugs in the C library; in the Kernel; all in code you cannot see either because it is locked/compiled, or because it's in a different file you didn't bother to read. And so on. And yet the world didn't stop spinning, and many people use close() every day without knowing or dealing with a myriad of bugs and weird corner cases[1]. So it is that even if code hiding is sometimes bad, it is sometimes less bad than other things we could be doing.
> Nothing is being abstracted here either- we're still only talking about giving this algorithm a name
I would say giving something a name that summarizes it so you can refer to it by the name as shorthand (and in code, "referring to" means "using")... is in fact exactly what "abstraction" is, it's possibly almost a good working definition of "abstraction" in fact.
Abstraction in mathematics is the process of extracting the underlying essence of a mathematical concept, removing any dependence on real world objects with which it might originally have been connected, and generalizing it so that it has wider applications or matching among other abstract descriptions of equivalent phenomena
I absolutely do not mean "refer to it by the name as shorthand", and I believe such a definition to be incredibly unhelpful: Some languages do not have variables (or do not need them), and yet they still have very powerful expression-building capabilities useful in abstractions, however I urge you to distinguish between "giving something a name" and "abstracting" something for a much more important reason:
We can waste a lot of time looking for a good name, or we could waste a lot of time looking for a good abstraction. The latter will always pay a higher return than the former, so I do not think we should make it easy for young impressionable minds to confuse these two things.
It's not about the name, it's about the boundaries of the concept, but how do humans establish a concept and it's boundaries without giving it some kind of name to refer to?
But I am may not be familiar with the kinds of languages you speak of, where there are no variables (and no functions/procedures? No names at all?) but "still have very powerful expression-building capabilities." I'm not totally sure what you're talking about, i admit.
In the OP, describing a pretty strictly procedural/functional environment, I think taking logic and putting it in a procedure with a name is the fundamental tool of abstraction offered by such an environment.
> I am may not be familiar with the kinds of languages you speak of, where there are no variables (and no functions/procedures? No names at all?) but "still have very powerful expression-building capabilities." I'm not totally sure what you're talking about, i admit.
I venture you probably are familiar with them (even at least somewhat), but you might not think of them as languages. Or perhaps you are turned off because they are oblique.
Regular expressions are a good example, because they're really common. Here's a program I use frequently:
"([^"\\]*(\\.[^"\\]*)*)"
Incidentally, this shows off an interesting and useful power of grouping. Too many people learn grouping and capturing at the same time, and capturing seems so much more useful that the second use of parenthesis is missed.
Another good example to consider is pointfree Haskell[1]. This is typically sold to programmers as a kind of golf, but this combinational style can make it possible to spot optimisations that are difficult in a pointful implementation. If you don't like types, you also see a lot of this in languages in the Forth family: Simple "improper" functions like : K 1024 * ; are really common, with no effort at all to give names to arguments of short functions, but people sure do like their stack diagrams...
> It's not about the name, it's about the boundaries of the concept, but how do humans establish a concept and it's boundaries without giving it some kind of name to refer to?
Consider the following function:
def m(f,a): [f(x) for x in a]
It is very easy to come up with a name for "m" but it is difficult to come up with a useful name for "f" or "a", so we typically don't. In APL we call f ⍺⍺ or ⍵⍵, and a we call ⍺ or ⍵, and you typically do not name your arguments at all (you can, of course), but it's very liberating not having to think of names for these things! When using other languages, I will typically name function-arguments f or g, and non-functional arguments x and y, but this is just a convention I use.
Of course we would never need to write m in APL because it's just ¨
> I think taking logic and putting it in a procedure with a name is the fundamental tool of abstraction offered by such an environment.
Your brain is the fundamental tool of abstraction!
If you want to create a ledger for tracking deposits and withdrawals from an account, you may build an abstract transaction log that can contain any values - and you could just as easily be able to use it for a bank account as for a shopping cart or fuel tracking in an airplane. And yet, being "too abstract" may mean the code is too hard to use (successfully, or it's too slow, or whatever) - it might be so much easier to build something simpler and more concrete, even if it means you have to write "basically the same thing" when you decide you're done making bank accounts and it's time to start making shopping carts.
The thing to keep in mind is that it's you deciding when it's easier and when it's harder. That's why so much of programming seems subjective, and people have strangely confusing rules, because we don't all read code the same way; we don't all have the same software experiences that lead us to think a certain way.
Nothing is being abstracted here, and nothing is gained by indirection
Making the rest of the function readable would be a win. If the two functions in the if-then are quite long, and if that clause appears in many places, then that could be a big win in the aggregate.
Such indirection requires readers to go look up the definitions of other things to understand the meaning of each function.
The nice thing about abstraction is that we don't need to go look up the definitions (unless we're tracking down a bug that's in those parts!): we can read a line like `if checkHasPermission(x):` and get the gist of what's happening, and skip over it if we don't care about the permission system at the moment.
> The motivation for many splitting out a prime-testing function is that primality testing is hard to do efficiently
Yes. Also that there are many ways to do it, the details of which you don't necessarily care about.
> Nothing is being abstracted here
It absolutely is. To abstract is to ignore differences. Here you are abstracting over the methods used to do primality testing. If you are using "1&p:" in J to test if something is prime, you are saying something like: "I don't care exactly how it's done, just that it uses one of the reasonably fast and correct algorithms available."
> nothing is gained by indirection: Does this is-prime routine even deserve a name?
I am so baffled by these claims that I wonder if I'm missing your point.
My reaction is: A tremendous amount is gained by this abstraction. It is exactly the abstraction you want most of the time. And yes, it deserves a name, because "Is it prime?" is how everyone thinks about it and talks about. It links up with the whole history and culture of mathematics.
The J authors did (correctly imo) think that it was. It's called ">:".
> isPrime =. 1&p:
In this case it does not, although I'd argue it's not so much about "isPrime" not being a useful concept worth naming on its own, as that J is a dense language working with a limited character set and there are a bunch of related concepts around "primeness" that need to packed into the "p:" verb -- accomplished in this case allowing the left argument to access a bunch of different verbs worth naming. Said another way, "1&p:" just happens to be the way J names this concept, for reasons of economy.
For the prime case in particular, it will be a named function in most math libraries that deal with such things, or languages where such math is central to the domain:
I am still unclear what your claim is: that such functions are always an example of "too much abstraction"? If so, I don't think that claim is tenable.
> The J authors did think that it was. It's called ">:".
So did CL. I still disagree though: >:/ is different than (1+])/. 1++:i. is the same number of characters as >:+:i. I feel like there are many more valuable functions more deserving than this one.
> I am still unclear what your claim is: that such functions are always an example of "too much abstraction"? If so, I don't think that claim is tenable.
Sorry it's so difficult to figure out; You're not asking really constructive questions, so maybe one of my other replies will help. I'm not saying anything is always one way or another because I think the only absolute is that it always depends!
When the author used generic "foo" examples, it's hard to tell if there's meaning behind it or not. In a real example, there might very well be meaning behind it, perhaps his "foo" examples in his actual life are more like your "checkHasPermissions" example.
But I think you're right that "abstraction" is the important concept here, which "indirection" is either a synonym for or one part of, depending on what you mean by "indirection".
I think the main challenge of organizing your code ("architecture") is figuring out the right abstractions.
And "too much abstraction" is definitely a thing.
In my experience there aren't any magic rules or guidelines you can just follow to get the right level and kinds of abstraction. There are guidelines and things to think about, and while getting more experience writing (and maintaining!) code is the only way you get better at it, reading and thinking and discussing it helps too.
Some things that have been attempts to guide people to the right levels and kinds of abstraction, like "design patterns", have proven to be no magic bullet either.
In actual experience, I think most of us end up erring on the side of too much/too complicated abstraction ("over-engineering"), so when it doubt, it makes sense to try erring on the side of less abstraction, including duplicating code at times (do not let "DRY" be your only guide).
Which, in the end, is what I think OP is telling us.
> In actual experience, I think most of us end up erring on the side of too much/too complicated abstraction ("over-engineering"), so when it doubt, it makes sense to try erring on the side of less abstraction, including duplicating code at times (do not let "DRY" be your only guide).
Yeah, this. I've noticed a lot of younger engineers(myself included when I was starting out) have a tendency to try to avoid duplication at all costs, even when it's unclear whether the two pieces of code will evolve in the same way over time.
So you wind up with a single class but then a year later you have to rip the single class out and replace it with the repetition of the code, and then make a change that makes the two cases no longer repetitious. It would have probably been better to repeat the case.
My heuristic for deciding whether to replace duplication with abstraction is whether I've copy-pasted the same code at least three times, and/or it is code where eliminating the abstraction is trivial. So if I can replace the code with a simple function I'm more apt to do it than if I would have to create a class or composition of classes to do the same thing that the duplication does. It's way more maintainable at that point to just leave the copy/paste in and deduplicate when there's actual need to create a new tool in the code base.
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.
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.
>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.
I find your example actually being a counter-example. The comment should rather be at the top of the definition/implementation of is_legacy_user, so IDEs and users can read and extract it when neccessary (thinking of Javadoc/Python's docstrings). The obvious advantage is that you want to keep comment and function in sync, and not all the calling points distributed around the code. Also, your example is a counterexample because the function name should be spelling, and a comment like
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."
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").
I don’t quite get the point of article. It essentially boils down to:
“Abstractions — or rather, indirections — are bad for for readability, but they are good in some cases.”
Umm... this can be said for pretty much anything. Over use of anything is bad. Inappropriate use of anything is bad.
> Mostly, I want authors to be aware that there is a human cost to indirection that is felt more acutely by everyone reading the code except the original author.
Sure. It’s a well known fact that abstractions have associated costs — nothing new there. However, the author seems to forget that not abstracting things can be equally costly — even when human readability is concerned.
Imagine someone trying to figure our the business logic be reading code. All they need to know is some conditions are being met for certain steps to be executed. The actual steps involved in the check are irrelevant to the business logic. The condition can be modelled separately from the business logic and can even change independently. For example:
In this case, it just happens that the satisfaction of a condition is implemented as prefix check. In the future, it might be changed to checking a flag or reading a database, or anything really. If you sprinkle the startswith() check all over you code, it will be hard to make the change. And it actually hurts readability. While reading a large codebase, you want to focus on specific parts of it — getting into the details of everything all at once does not make it easier.
The most important thing to note is — programming is not confined to s strict set of rules. It’s not an exact science. You can’t define a strict a rules that must be followed. What makes sense somewhere is completely useless elsewhere. The Linux Kernel is full of goto statements, despite the fact that goto is considered harmful.
Dogma Driven Programming must be avoided. Even well known programming practices are guidelines at best. One must evaluate whether a given rule fits a given situation.
Coming back to the example at hand, functions are meant to divide the program by the logical tasks being performed, not by number of lines or any other metric. As I said before, it’s more art than exact science.
Is the check for foolikeness of something a logically distinct task? It depends on the context. There is no way to write a cardinal rule either way.
PS. I sincerely hope no one comes to me for code review with a monolithic function because of all the inlining — citing this article as inspiration.
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 make debugging easier as you can test both those pieces independently. What the OP calls indirection is just bad code or meaningless abstraction.
Zed Shaw puts it well: "Abstraction and indirection are very different yet cooperating concepts with two completely different purposes in software development. Abstraction is used to reduce complexity. Indirection is used to reduce coupling or dependence."
Yup, seems like the author had trouble following badly written abstraction and/or indirection and came up with the idea that they are bad.
Abstractions are invaluable if you are dealing with a large codebase. Even the original author can’t keep everything in their head.
Indirections are invaluable if you want to be able to modify code in the future without making a gigantic mess — and potentially breaking compatibility in the process.
The wrong abstraction can be worse than no abstraction at all. And it can be non-obvious if you've got the "wrong" abstraction until much later when it's been in use for a while and had to be maintained -- and sometimes even then, we aren't good at recognizing that our pain is coming from the use of the wrong abstractions.
We often are, without necessarily realizing it, reluctant to refactor existing abstractions once there, preferring to alter them slightly or even more add new ones on top (which can make things worse in the long-run). And this isn't necessarily irrational, refactoring/removing existing abstractions is expensive, and with no guarantee you'll get it "right" this time either.
Mechanically extracting things as in OP can often lead to "wrong" abstractions, which is what I get as the takeaway from the OP, which is a good reminder.
You are right that abstraction is the main tool we use as computer programmers, one way or another. Thinking in abstractions is the main thing we do that characterizes computer programming in contrast to other endeavors. So we certainly can't give it up entirely.
But this fact, I think, from my observations over my career, often leads us to over-abstraction ("over-engineering"), and the wrong abstractions. Because we are so taken with abstraction (it is neat, and you probably think so if you enjoy computer programming). It is good to be reminded that there is such a thing as too much as well as too little abstraction. Sometimes it's the "wrong" abstraction, but sometimes the "right" abstraction and the time it would take to arrive at it were unnecessary, and less/no abstraction would have served you just fine.
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.
Thanks for sharing Zed Shaw's wisdom. I will chew on it.
"Abstraction is used to reduce complexity."
My taxonomy: Abstractions hide complexity (bad), mental models simplify (good).
I'll consider how Shaw's worldview relates to my own.
"Indirection is used to reduce coupling or dependence."
My taxonomy: Indirection defers a design choice. A la Design Patterns.
One of my heuristics, when weighing alternatives, is to pick the design with the shallower stack depth. I'm just not smart enough to keep track of many moving parts.
FWIW: I see the 'isPrime' function more as macro (shorthand) done for legibility than a potent future cutpoint (eg Strategy).
219 comments
[ 2.7 ms ] story [ 60.0 ms ] threadI 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 forgot that it's about telling the computer to do something, as clearly as possible.
Or alternatively, it's to explain to the next reader of the code, as clearly as possible, how the problem was solved (in such a way that it can also be executed by the computer).
[1] http://www.cs.kent.edu/~jmaletic/Prog-Comp/Papers/fix-1993.p...
Imagine if you have to check permission to do something, and that check is really complex and long code.
If would be better to have code like: if (hasPermissionToEdit(myUser)){ ...
Than a ton of ANDs and ORs in that if, that nobody would quickly recognize as a permission check... even if it's just there once.
As a minimum I'd put that in a properly named variable first, if I really wanted to avoid an extra function.
That's not completely the point of the article, but both issue are interesting metric for deciding the level of factorization.
Also one need more accurate specification when defining a call deepness alarm. Most direct call to any random API will probably already explode a limit of 3, which doesn't necessarily say that the framework is brokenly atomized.
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.
A good reason would be to clarify the why or the what by "glossing over" the how:
Yes, you're "hiding" the actual steps (the "how"), but in doing so, you're elevating your intent (the "why"/"what") to the forefront, and so you're making it clear what your code is supposed to be doing. This makes it easier to understand overall, and makes it easier to determine when your implementation and your expectations don't match (because your expectations are clearly described).A bad reason for indirection is one that's super-common in Java: namely, "we might need to make this swappable at some undetermined point in the future". It's the kind of thing that leads to
That's just repetition for reasons that are at best forced by technical limitations, and at worst by paranoid decision-making.A mark of skill at coding (in particular, as a part of software development) is the ability to clearly convey intent to the reader. Sometimes, that means bringing a new concept into code that makes the problem easier to understand and talk about (to borrow an example from elsewhere in the comments here, introducing a DateInterval type can make common operations on two dates clearer). Sometimes, that means recognizing when the details get in the way of the point.
We often do the same things when we write prose for humans to read. When writing prose for humans, it may help to introduce new clarifying terms like "time complexity" and "memory usage" so that we're not constantly explaining that when we say "performance" this time we mean "performance specifically in terms of how much RAM is used" and that time we mean "performance in terms of how much time is required to for the function to complete". It certainly often helps to edit down overly-detailed explanations that distract from the main point (possibly putting the fuller explanation into a footnote); if someone asks where you've been, how helpful is it to tell someone that you opened your garage door, got into your car, started your car, shifted gears into "drive" (or 1st gear), drove down your driveway, turned left....etc, as opposed to telling them that you went to the store to buy milk?
The point I'm getting at here is that telling someone "avoid indirection" is a bit like telling someone "avoid summarizing." Sometimes, a full account is needed, sure -- like if you're on a witness stand -- but most of the time, summary is one of many useful tools to convey information clearly.
Likewise, sometimes it's necessary to see every single explicit step being performed -- when doing in-depth performance optimization, for example -- but most of the time, indirection (especially by functionalization) is one of many useful tools to convey information clearly.
Indeed, indirection in code is actually "safer" than summarization in writing. In code, you can always "go to definition", where in writing you may not have that option.
In terms of when to split them out, my rule of thumb is "when there's more than one implementation class that does the same function".
Good examples are things like Comparable/Comparator, Reader, Writer, Serializable -- these are all descriptions of behavior for which multiple implementations exist. Interfaces, in general, are adjectives (or at least adjective-ish).
Bad examples are things like CheckoutService, ShoppingCartManager, JwtTokenManager -- these are all descriptions of specific components within an application that have a single implementation. These are nouns, and generally they're a sort of "proper noun" (in that they refer to one single thing by its name). If you don't already have many implementations for the behavior they implement, then you don't need an interface (since an interface is just a way of describing a set of "external-facing" contracts which many specific implementations may fulfill -- like List vs LinkedList/ArrayList).
The reason why is simply that it's just waste. It's more code for more code's sake. At best, you're just clicking "go to definition" one extra time, and at worst your code is confusing (because someone might assume the presence of the interface means it's "swappable" behavior, rather than integral application logic).
The "YAGNI" ("You Aren't Going To Need It") principle applies here: if you don't know you're going to need it, don't build it. Once you do know that you're going to need it, then you build it. Anything before that point winds up being wasted time/effort.
Also, it is very refreshing to read about and discuss actual development.
https://web.archive.org/web/20130227000541/http://defect.ope... https://github.com/oracle/solaris-ips/commit/53bb14f750dd111...
Abstraction has a very specific implementation, usually polymorphism and a very specific intent. It's not just a synonym for nested function calls or helper functions or one stop shop facade or whatever.
Visual Studio has this and it's called "Peek at function".
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 change is supposed to affect them all and that quickly becomes an exponential problem.
So much of this is cargo culting attempts to keep methods short without understanding why long methods are a problem in the first place. That problem is the amount of state you have to juggle mentally, short abstractions like this that don't eliminate that state, so they create no benefit and the cost of obfuscating the code.
Yes. Please. Stop. Doing. That.
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 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.
In the original example, what exactly it means to be “foo like” is not obvious. You need to bounce into the other file/method in order to understand it’s purpose.
In good abstractions, like the ones you describe, you can read the method and understand it’s intended purpose as is. In fact, a good abstraction will be easier to understand than inlining all it’s details.
This is a semantic point, but I don’t think there really is such a thing as good indirection. Indirection is the result of bad abstractions. The author is basically just saying “don’t make bad abstractions”.
I’m making more of a semantic point than anything else. If you define indirection as anything which causes you to jump from the code directly in front of you at any point, then yes, you can call even good abstractions indirection.
I’d argue that such a definition makes the word “indirection” must less useful, as it becomes pretty much synonymous with “abstraction”. I think it’s better to reserve the word “indirection” to describe abstractions that require you to look elsewhere to understand what the abstraction actually is.
Everything is indirection.
E.g.:
is, I presume, the sort of thing the author means to suggest.If I was doing a code review and came to this:
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
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/
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`
vs (edit) for formattingYes, 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 intention of the function", because if the function doesn't do what it's intended to do, that's a bug.
The reason I say I don't think you understand what you're saying is that none of what you're actually saying applies to the examples you're giving.
`is_foolike` doesn't describe the intention of the method. It gives an entirely vague and inaccurate view of what the method does. So even though we both agree that functions should describe the intention of the method, you're saying `is_foolike` is an okay name even though it doesn't do what you say it should?
`starts_with_foo` describes the intention of the method. It doesn't describe the implementation: `starts_with_foo(x)` might expand to `x.startswith('foo')` or `x[:3] == 'foo'`, but I don't care which, because the name accurately describes what it does either way.
Your example doesn't elucidate. 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 our "high-level, in your business domain", the program is so badly tangled that we're not going to get any truths about good programming from it.
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 foo" isn't hiding implementation details, it's being opaque about what a function does.
Put another way, "what a function does" isn't its implementation. "How a function does what it does" is its implementation.
> `starts_with_foo` implies an assertion a string beginning with a particular prefix
Yes, exactly. Because that is what it is intended to do, we hope, since that's what it does. That's NOT the implementation: there are plenty of different ways to implement testing whether a string starts with 'foo', and the name `starts_with_foo` isn't coupled to any of them.
> The point being that even though, in this (again) entirely fictitious example, the intention of the method is different from it's implementation
Yes. `starts_with_foo` is also different from the implementation, while still describing the intention of the method.
Could you explain to me why you think `starts_with_foo(x)` describes the implementation `x.startswith('foo')` and not some other implementation (such as `x[:3] == 'foo'`)?
If that’s acceptable, why is an IsFooLike which only checks one condition unacceptable, even though it (imo) expresses the same intention?
The idea with naming is to capture as much of the meaning of the function as possible, and 'isFooLike' doesn't really capture any meaning (what does it mean to be like a foo?). In business, there are typically names for the topics, like `is_profitable_transaction` or `meets_sales_targets`, which may indeed encompass some very complex logic, but are a coherent idea, so naming is usually a bit easier than this. But you're right, sometimes the requirements are bad and you don't have a chance to go back to requirements before a deadline. In those cases, I don't think the naming matters much because no name you come up with is going to represent the concept. So I guess `is_foolike` might be the best you could come up with, but it's certainly not good code. I'd also be less likely to pull out a function in the first place because it's a premature abstraction: if the abstraction isn't coherent enough to be named well, we probably don't understand it well enough yet to abstract it out.
> If that’s acceptable, why is an IsFooLike which only checks one condition unacceptable, even though it (imo) expresses the same intention?
It's not the same intention.
In the more complex case, you have a bad name because the name doesn't describe the (overly complex) intention of the code, but there isn't a better one.
In the second case, you have something that does a clear thing, so you should name it what it does. For a more complex case, you're not going to be able to capture every nitpicky detail of what the function does in the name, so you have to describe it in a broader concept. But with a short simple function like this, there really isn't an excuse for using a vague description.
I think you're still ignoring my explanation that what a function does and intends to do is different from how it does it. So I'm going to again insist that the two of you answer these questions to prove they actually understand my points before disagreeing with me:
1. I'm claiming that in correctly-working code, the intention of the code IS what it does. Is there a case where code would do something other than it's intended to do, and this isn't a bug or at least misleading code?
2. I'm claiming that what a function does is not the same as its implementation. Why are you claiming `starts_with_foo(x)` coupled to the implementation `x.startswith('foo')` and not `x[:3] == 'foo'`?
A name that is long enough to tell you all you might need to know about what the thing named does is too long to be usable at all.
A name needs to be long enough to distinguish it from the other things being named. That's what naming means. If it also gives you a hint about which thing, of the things named, it really does, that makes it perfect.
Anything beyond that adds cognitive load, making it exponentially worse with each syllable added.
In addition, if we for some reason want to change what we mean with foo-like (say to "contains("foo"), we suddenly have to change all calls to the function, in addition to the defintion.
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?
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?
Nothing in `starts_with_foo` describes what commands are being used--it describes what it does, not how it does it. With your logic, telling what the function does at all is too low a level.
> 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.
Okay, so why not just name your functions, `aaaaa`, `aaaab`, `aaaac`, etc.? If the name of the function is `cdyfj`, then the definition of the function will be the authoritative place in the code that tells us what `cdyfj` means.
Aren't you worried that `isFooLike` gives us too much information about the implementation? After all, the string "foo" is in the implementation. What if you want to test for "bar" later? Then you'll have to change all the places where `isFooLike` is used, in addition to the definition!
> What is a bug?
1. You're claiming that the name should tell us what the intention of the function is.
2. You're claiming that the intention of the function isn't `starts_with_foo`.
3. The function tests to see that the string starts with 'foo'.
The only way all three things can be true is that the intention is if the function does something it's not intended to do. That's the definition of a bug.
> What do we mean when we say that something is Foolike?
The fact that nobody could possibly answer this question is exactly the problem I'm pointing out. You might as well just name the function `cdyfj`, because `isFoolike` doesn't actually give you much more information than `cdyfj` does.
No, we need to say what we are doing on the right semantic level for that part of the program. It is rarely describing exactly what we are doing, because then only function of functions will be to abbreviate common patterns of code. The main function of functions is to abstract. At this particular level, Foolike has a semantic meaning to the reader of the program who knows what a Foo is and what isn't. And in this particular version of the program we happened to implement it from the first characters of the string. But maybe later we decide that it anything that inherits from a Foo, or has the Foo interface, or just walks, but not necessarily quacks, like a Foo.
And the reason I don't call the function some nonsense is because then it is hard for the readers of the code to understand the meaning of it. And it might not help even if the they read the definition of the function, because the definition isn't the meaning. It doesn't say anything about why it is interesting to know if the string starts with 'Foo' or not (which is, as I have mentioned, the problem with the name you suggested).
>> What do we mean when we say that something is Foolike?
>The fact that nobody could possibly answer this question is exactly the problem I'm pointing out.
No, we already know what foolike means. It is part of the domain that we are working with. The people who wrote the code and the people who read the code are assumed to have enough domain knowledge to know what a Foo is, and they understand me when I say things "we better use a Foolike here, otherwise we'll get a Bar problem".
Let's use a bit less abstract example. Say I implement the function NextInvoiceNumber(). My first implementation might be something like {return ++n} (because I'm making a proof of concept and I'm in a hurry). You seem to argue that a better name of the function would be something like AddOneToNAndReturnN(). But then the reader of the code won't understand what the purpose of the function is, because nothing says anything what n is good for and why we are interested in getting the next n. And the moment I start to add different number series for different articles and some of them increment with 10 and some with 100, the name AddOneToNAndReturnN() will be wrong. That will not be a problem for NextInvoiceNumber(), because that is perfectly normal for anyone who has done any work with invoicing. Invoicenumbers are part of the domain that the developer must know about.
It sure doesn't. Maybe readers are much smarter than me, but I don't know that it means to be foolike until I read the function definition. Apparently something is foolike if it is a string starts with 'foo', but I certainly wouldn't be able to guess that from the name. And I still don't know what a Foo is or isn't.
> But maybe later we decide that it anything that inherits from a Foo, or has the Foo interface, or just walks, but not necessarily quacks, like a Foo.
Okay, so you're naming this based on what it might do in the future rather than what it does? How are you predicting the future here? And when it does the future thing, will you change the name so that it is talking about the future again, since you have such an objection to names referring to what a function presently does?
> And the reason I don't call the function some nonsense is because then it is hard for the readers of the code to understand the meaning of it.
The concept of being like a food IS some random nonsense: it's hard for readers of the code to understand the meaning of it.
> Let's use a bit less abstract example. Say I implement the function NextInvoiceNumber(). My first implementation might be something like {return ++n} (because I'm making a proof of concept and I'm in a hurry). You seem to argue that a better name of the function would be something like AddOneToNAndReturnN(). But then the reader of the code won't understand what the purpose of the function is, because nothing says anything what n is good for and why we are interested in getting the next n. And the moment I start to add different number series for different articles and some of them increment with 10 and some with 100, the name AddOneToNAndReturnN() will be wrong. That will not be a problem for NextInvoiceNumber(), because that is perfectly normal for anyone who has done any work with invoicing. Invoicenumbers are part of the domain that the developer must know about.
Okay, I'm guessing you mean something more like:
And that's perfectly reasonable, because getNextInvoiceNumber describes what the function does.A name like addOneToInvoiceNumberAndReturnInvoiceNumber would be bad, not because it's inaccurate, but because it doesn't tell you what the function does in context.
We don't have any similar context to concern ourselves with for 'starts_with_foo'.
It seems like going back to the beginning of the discussion, you guys assigned some meaning in your heads to the adjective "foolike" that you didn't choose to share with me and decided based on this meaning in your head that I was wrong. Perhaps the name `is_foolike` is a better name within the imaginary context you've created, but I was talking about the code that was linked on this thread, not the code you've imagined and didn't tell me about.
The big problem I have with the toy example is that both pieces of code convey different ideas to a human reader, and limiting myself to the situations the author finds the indirection cumbersome, such as code reviews, or debugging, both would lead the reader in different directions. The suggested solution would make a reader think the business requirement of that code is to check if the input starts with foo, whereas the indirection code suggests to the reader that the input should be like foo (which in this case happens to be by testing it starts with foo, but that may be incorrect, or may change).
I think the author’s example really hurts their case. A better example may be where the intention indeed is to check whether the input starts with the string foo, but you want to trim the sample first.
I’ve seen code that would abstract that out with a function trimmedStartsWith, or something, instead of just input.trim().startsWith(foo), where I think the former only makes sense if you’re doing it a bunch of time.
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.
Sure it is some effort to backtrack to the util functions, but I think the effort is worth it because I would rather have a developer spend multiple hours debugging and figuring out the code than having something that is a nightmare to iterate upon.
A Philosophy of Software Engineering:
https://www.amazon.com/Philosophy-Software-Design-John-Ouste...
This is mentioned as one of the symptoms/appearance of bad code: Shallow/Passthrough methods, which results deep call chains, adding up cognitive overload. The author in this book recommends deep module over shallow module, which tends to end up with more cohesive code.
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 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. Peeking a function with 5 lines, peek it. If it’s larger, jump to it.
Remember although code is interpreted by machines it's read by humans...
In older code, or code worked on by a larger team, I always chase into functions to understand the code, and don't rely on names. I'm also less inclined to indirect code unless the abstraction has genuine meaning, rather than being a compression technique for repetition, no matter how well named.
But like the author said, the reuse fallacy prevents this from having a good RoI until there is a significant usage of said abstraction.
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.
It isn't clear whether the "check" prefix means the function returns based on whether the checking was successful or not.
There's no iron law or mathematical proof that says this is optimal. It's just a preference some people have. Exceptions are the error handling mechanism for many programming languages. Authorization errors are a type of error. Under this construction, exceptions are the appropriate way to handle authorization errors, including when a principal attempts to access an resource they are not authorized to access.
By contrast, the performance cost of capturing stack traces when there's an authorization error is insignificant. How many authorization errors do you have per second in normal operation? For many systems, the answer is down in the millibecquerels to microbecquerels.
Under these circumstances, the balance leans quite heavily to the side of using exceptions to report authorization failures.
Even from a code U/X perspective this is better. How is someone going to know that x.starts_with("foo") means that the object is foo-like, rather than some subset comparison only? The name "is_foolike()" tells them unambiguously what information the function will return and more importantly, what the information means. Abstraction is about assigning higher order meanings to groups of information and behaviors so that we humans can keep the concepts straight in our minds, and is_foolike(x) is a lot easier to understand the underlying meaning of than x.starts_with("foo").
How you react to something being foo-like or not is a question for a higher abstraction level to decide, and present a unified front from there, like get_foolike_items(y), which in turn is needed by get_high_value_targets("sweden") (because all high value targets have foo-like metadata), which is in turn passed to launch_nuclear_missiles(targets).
However for code reviews and debugging an engineer must actually read and understand the 'how' to prove and disprove the 'what'.
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.
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.
(Also known in Python as duck typing when applied to types.)
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?
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.
All the same I think you may have thrown out the abstraction with the bath water.
> Does this is-prime routine even deserve a name?
Absolutely. If you saw it over and over you might start just reading it as "is-prime" instead of going through the process of mentally interpreting it each time. But what about similar-looking functions with similar functionality? You want a programmer to go and make you a promise that it has the meaning (if not the implementation) you're thinking about. That promise is the same as "code-hiding".
Even though we're unlikely to need this construction more than once:
This one (or one just like it) appears quite common: But Guido already noticed that! That's why he made an "in" operator in the first place! As soon as our routines become too generic, we're just making APL without arrays, and that's why taste matters so very much.> That promise is the same as "code-hiding".
If I can see the implementation at the same time as its usage, then the code is not hidden. Ipso facto.
But to expand more on what I mean by this, consider that close() is hiding code: You get all of the bugs in the C library; in the Kernel; all in code you cannot see either because it is locked/compiled, or because it's in a different file you didn't bother to read. And so on. And yet the world didn't stop spinning, and many people use close() every day without knowing or dealing with a myriad of bugs and weird corner cases[1]. So it is that even if code hiding is sometimes bad, it is sometimes less bad than other things we could be doing.
[1]: Ha ha, only serious. http://geocar.sdf1.org/close.html
I would say giving something a name that summarizes it so you can refer to it by the name as shorthand (and in code, "referring to" means "using")... is in fact exactly what "abstraction" is, it's possibly almost a good working definition of "abstraction" in fact.
Abstraction in mathematics is the process of extracting the underlying essence of a mathematical concept, removing any dependence on real world objects with which it might originally have been connected, and generalizing it so that it has wider applications or matching among other abstract descriptions of equivalent phenomena
I absolutely do not mean "refer to it by the name as shorthand", and I believe such a definition to be incredibly unhelpful: Some languages do not have variables (or do not need them), and yet they still have very powerful expression-building capabilities useful in abstractions, however I urge you to distinguish between "giving something a name" and "abstracting" something for a much more important reason:
We can waste a lot of time looking for a good name, or we could waste a lot of time looking for a good abstraction. The latter will always pay a higher return than the former, so I do not think we should make it easy for young impressionable minds to confuse these two things.
But I am may not be familiar with the kinds of languages you speak of, where there are no variables (and no functions/procedures? No names at all?) but "still have very powerful expression-building capabilities." I'm not totally sure what you're talking about, i admit.
In the OP, describing a pretty strictly procedural/functional environment, I think taking logic and putting it in a procedure with a name is the fundamental tool of abstraction offered by such an environment.
I venture you probably are familiar with them (even at least somewhat), but you might not think of them as languages. Or perhaps you are turned off because they are oblique.
Regular expressions are a good example, because they're really common. Here's a program I use frequently:
Incidentally, this shows off an interesting and useful power of grouping. Too many people learn grouping and capturing at the same time, and capturing seems so much more useful that the second use of parenthesis is missed.Another good example to consider is pointfree Haskell[1]. This is typically sold to programmers as a kind of golf, but this combinational style can make it possible to spot optimisations that are difficult in a pointful implementation. If you don't like types, you also see a lot of this in languages in the Forth family: Simple "improper" functions like : K 1024 * ; are really common, with no effort at all to give names to arguments of short functions, but people sure do like their stack diagrams...
[1]: https://wiki.haskell.org/Pointfree
> It's not about the name, it's about the boundaries of the concept, but how do humans establish a concept and it's boundaries without giving it some kind of name to refer to?
Consider the following function:
It is very easy to come up with a name for "m" but it is difficult to come up with a useful name for "f" or "a", so we typically don't. In APL we call f ⍺⍺ or ⍵⍵, and a we call ⍺ or ⍵, and you typically do not name your arguments at all (you can, of course), but it's very liberating not having to think of names for these things! When using other languages, I will typically name function-arguments f or g, and non-functional arguments x and y, but this is just a convention I use.Of course we would never need to write m in APL because it's just ¨
> I think taking logic and putting it in a procedure with a name is the fundamental tool of abstraction offered by such an environment.
Your brain is the fundamental tool of abstraction!
If you want to create a ledger for tracking deposits and withdrawals from an account, you may build an abstract transaction log that can contain any values - and you could just as easily be able to use it for a bank account as for a shopping cart or fuel tracking in an airplane. And yet, being "too abstract" may mean the code is too hard to use (successfully, or it's too slow, or whatever) - it might be so much easier to build something simpler and more concrete, even if it means you have to write "basically the same thing" when you decide you're done making bank accounts and it's time to start making shopping carts.
The thing to keep in mind is that it's you deciding when it's easier and when it's harder. That's why so much of programming seems subjective, and people have strangely confusing rules, because we don't all read code the same way; we don't all have the same software experiences that lead us to think a certain way.
Making the rest of the function readable would be a win. If the two functions in the if-then are quite long, and if that clause appears in many places, then that could be a big win in the aggregate.
The nice thing about abstraction is that we don't need to go look up the definitions (unless we're tracking down a bug that's in those parts!): we can read a line like `if checkHasPermission(x):` and get the gist of what's happening, and skip over it if we don't care about the permission system at the moment.
I will be the first to admit that proper-naming is most usually rather challenging .
Yes, of course, but that (I claim) implies that the function/method has abstracted something.
The parent was claiming that using indirection to hide code can be useful for readability, even when there is nothing being abstracted.
Yes. Also that there are many ways to do it, the details of which you don't necessarily care about.
> Nothing is being abstracted here
It absolutely is. To abstract is to ignore differences. Here you are abstracting over the methods used to do primality testing. If you are using "1&p:" in J to test if something is prime, you are saying something like: "I don't care exactly how it's done, just that it uses one of the reasonably fast and correct algorithms available."
> nothing is gained by indirection: Does this is-prime routine even deserve a name?
I am so baffled by these claims that I wonder if I'm missing your point.
My reaction is: A tremendous amount is gained by this abstraction. It is exactly the abstraction you want most of the time. And yes, it deserves a name, because "Is it prime?" is how everyone thinks about it and talks about. It links up with the whole history and culture of mathematics.
The J authors did (correctly imo) think that it was. It's called ">:".
> isPrime =. 1&p:
In this case it does not, although I'd argue it's not so much about "isPrime" not being a useful concept worth naming on its own, as that J is a dense language working with a limited character set and there are a bunch of related concepts around "primeness" that need to packed into the "p:" verb -- accomplished in this case allowing the left argument to access a bunch of different verbs worth naming. Said another way, "1&p:" just happens to be the way J names this concept, for reasons of economy.
For the prime case in particular, it will be a named function in most math libraries that deal with such things, or languages where such math is central to the domain:
https://reference.wolfram.com/language/ref/PrimeQ.html
I am still unclear what your claim is: that such functions are always an example of "too much abstraction"? If so, I don't think that claim is tenable.
So did CL. I still disagree though: >:/ is different than (1+])/. 1++:i. is the same number of characters as >:+:i. I feel like there are many more valuable functions more deserving than this one.
> I am still unclear what your claim is: that such functions are always an example of "too much abstraction"? If so, I don't think that claim is tenable.
Sorry it's so difficult to figure out; You're not asking really constructive questions, so maybe one of my other replies will help. I'm not saying anything is always one way or another because I think the only absolute is that it always depends!
But I think you're right that "abstraction" is the important concept here, which "indirection" is either a synonym for or one part of, depending on what you mean by "indirection".
I think the main challenge of organizing your code ("architecture") is figuring out the right abstractions.
And "too much abstraction" is definitely a thing.
In my experience there aren't any magic rules or guidelines you can just follow to get the right level and kinds of abstraction. There are guidelines and things to think about, and while getting more experience writing (and maintaining!) code is the only way you get better at it, reading and thinking and discussing it helps too.
Some things that have been attempts to guide people to the right levels and kinds of abstraction, like "design patterns", have proven to be no magic bullet either.
In actual experience, I think most of us end up erring on the side of too much/too complicated abstraction ("over-engineering"), so when it doubt, it makes sense to try erring on the side of less abstraction, including duplicating code at times (do not let "DRY" be your only guide).
Which, in the end, is what I think OP is telling us.
Yeah, this. I've noticed a lot of younger engineers(myself included when I was starting out) have a tendency to try to avoid duplication at all costs, even when it's unclear whether the two pieces of code will evolve in the same way over time.
So you wind up with a single class but then a year later you have to rip the single class out and replace it with the repetition of the code, and then make a change that makes the two cases no longer repetitious. It would have probably been better to repeat the case.
My heuristic for deciding whether to replace duplication with abstraction is whether I've copy-pasted the same code at least three times, and/or it is code where eliminating the abstraction is trivial. So if I can replace the code with a simple function I'm more apt to do it than if I would have to create a class or composition of classes to do the same thing that the duplication does. It's way more maintainable at that point to just leave the copy/paste in and deduplicate when there's actual need to create a new tool in the code base.
I cannot recall where I read it though -- it was quite a while back.
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.
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.
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:
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.
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.
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."
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").
“Abstractions — or rather, indirections — are bad for for readability, but they are good in some cases.”
Umm... this can be said for pretty much anything. Over use of anything is bad. Inappropriate use of anything is bad.
> Mostly, I want authors to be aware that there is a human cost to indirection that is felt more acutely by everyone reading the code except the original author.
Sure. It’s a well known fact that abstractions have associated costs — nothing new there. However, the author seems to forget that not abstracting things can be equally costly — even when human readability is concerned.
Imagine someone trying to figure our the business logic be reading code. All they need to know is some conditions are being met for certain steps to be executed. The actual steps involved in the check are irrelevant to the business logic. The condition can be modelled separately from the business logic and can even change independently. For example:
In this case, it just happens that the satisfaction of a condition is implemented as prefix check. In the future, it might be changed to checking a flag or reading a database, or anything really. If you sprinkle the startswith() check all over you code, it will be hard to make the change. And it actually hurts readability. While reading a large codebase, you want to focus on specific parts of it — getting into the details of everything all at once does not make it easier.The most important thing to note is — programming is not confined to s strict set of rules. It’s not an exact science. You can’t define a strict a rules that must be followed. What makes sense somewhere is completely useless elsewhere. The Linux Kernel is full of goto statements, despite the fact that goto is considered harmful.
Dogma Driven Programming must be avoided. Even well known programming practices are guidelines at best. One must evaluate whether a given rule fits a given situation.
Coming back to the example at hand, functions are meant to divide the program by the logical tasks being performed, not by number of lines or any other metric. As I said before, it’s more art than exact science.
Is the check for foolikeness of something a logically distinct task? It depends on the context. There is no way to write a cardinal rule either way.
PS. I sincerely hope no one comes to me for code review with a monolithic function because of all the inlining — citing this article as inspiration.
Edit: Typo correction.
Zed Shaw puts it well: "Abstraction and indirection are very different yet cooperating concepts with two completely different purposes in software development. Abstraction is used to reduce complexity. Indirection is used to reduce coupling or dependence."
Abstractions are invaluable if you are dealing with a large codebase. Even the original author can’t keep everything in their head.
Indirections are invaluable if you want to be able to modify code in the future without making a gigantic mess — and potentially breaking compatibility in the process.
Take care pursuing things for their own sake and not for a reason (I suspect you do but impressionable people are reading along with us).
The wrong abstraction can be worse than no abstraction at all. And it can be non-obvious if you've got the "wrong" abstraction until much later when it's been in use for a while and had to be maintained -- and sometimes even then, we aren't good at recognizing that our pain is coming from the use of the wrong abstractions.
We often are, without necessarily realizing it, reluctant to refactor existing abstractions once there, preferring to alter them slightly or even more add new ones on top (which can make things worse in the long-run). And this isn't necessarily irrational, refactoring/removing existing abstractions is expensive, and with no guarantee you'll get it "right" this time either.
Sandi Metz says:
> prefer duplication over the wrong abstraction
https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstracti...
Mechanically extracting things as in OP can often lead to "wrong" abstractions, which is what I get as the takeaway from the OP, which is a good reminder.
You are right that abstraction is the main tool we use as computer programmers, one way or another. Thinking in abstractions is the main thing we do that characterizes computer programming in contrast to other endeavors. So we certainly can't give it up entirely.
But this fact, I think, from my observations over my career, often leads us to over-abstraction ("over-engineering"), and the wrong abstractions. Because we are so taken with abstraction (it is neat, and you probably think so if you enjoy computer programming). It is good to be reminded that there is such a thing as too much as well as too little abstraction. Sometimes it's the "wrong" abstraction, but sometimes the "right" abstraction and the time it would take to arrive at it were unnecessary, and less/no abstraction would have served you just fine.
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.
If the means of combination is not created, the abstraction will end up being big and complicated, and non-extensible.
"Abstraction is used to reduce complexity."
My taxonomy: Abstractions hide complexity (bad), mental models simplify (good).
I'll consider how Shaw's worldview relates to my own.
"Indirection is used to reduce coupling or dependence."
My taxonomy: Indirection defers a design choice. A la Design Patterns.
One of my heuristics, when weighing alternatives, is to pick the design with the shallower stack depth. I'm just not smart enough to keep track of many moving parts.
FWIW: I see the 'isPrime' function more as macro (shorthand) done for legibility than a potent future cutpoint (eg Strategy).