6 ms·

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 thi

Okay, maybe it's just a poor example, but in the example used, the problem is definitely not too much indirection. If I was doing a code review and came to this: def is_foolike(x): return x.startswith("foo") I would comment, but my comment would be, "Could you name this function `starts_with_foo`?" This addresses both concerns mentioned in the article: 1. During review, when a reviewer is asked to verify that code is sensible before it can be merged into the main project. That reviewer probably has about a tenth as much time to spend as the original author does on that code. But if the reviewer comes to the code if starts_with_foo(x): do_something_with(x) They aren't likely to be misled in any way by the indirection. They can just keep reading, without having to look into the implementation of `starts_with_foo`, because either that name is accurate and they know what it does, or they'll discover that the name isn't accurate when they review the code. 2. While debugging future issues. This code will eventually be involved in a bug and some completely different developer will have to glance at this code to figure out what’s going on. They’ll have to understand some small section this code within a few minutes to determine what is relevant. They won’t be able to invest the time to understand the full thought process behind it, and a web of function definitions can slow down this process considerably. But again when they read the code, a good name means they can understand what the code does without having to read the implementation. I do think there's an argument to be made that indirection is a problem here, but it's a small problem compared to the enormous problem that `is_foolike` is a really bad name for that function. General rules for when to NOT pull something out into a function: 1. If it the function call wouldn't be clearer than the code (even a better name like `starts_with_foo(x)` loses some information contained in `x.startswith('foo')`, and the latter is readable enough that there's no real upside to the former. If the latter were even two lines long, it would become a lot more worth it. 2. If there's no repetition. Two similar pieces of code aren't enough: you don't understand from two use cases what pattern you're abstracting, so the result is just going to be a leaky abstraction. Three repeated pieces of code seems to be the sweet spot: now you have enough examples to know what's actually repeated, what should be arguments to the function, etc.[1] [1] https://blog.codinghorror.com/rule-of-three/ https://blog.codinghorror.com/rule-of-three/

justasitsounds 7y ago
`is_foolike` is a bad name if you name your methods to only describe how they are implemented internally. At a high-level, in your business domain, you generally write your code to hide the implementation details and to describe the intention of the method EG. `is_carbonated` if is_carbonated(beverage): beverage.jiggle(false) vs if beverage.startswith("co2"): beverage.jiggle(false) (edit) for formatting
- kerkeslager 7y ago
  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.
  - justasitsounds 7y ago
    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

justasitsounds 7y ago

`is_foolike` is a bad name if you name your methods to only describe how they are implemented internally. At a high-level, in your business domain, you generally write your code to hide the implementation details and to describe the intention of the method EG. `is_carbonated` if is_carbonated(beverage): beverage.jiggle(false) vs if beverage.startswith("co2"): beverage.jiggle(false) (edit) for formatting

kerkeslager 7y ago
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.
- justasitsounds 7y ago
  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