4 ms·

I really don't agree with no 11. My reasoning is that the reader should know at each line what conditions are assumed. Returning in the start is as anti-patter

by yonixw 8y ago

I really don't agree with no 11. My reasoning is that the reader should know at each line what conditions are assumed. Returning in the start is as anti-pattern for me as goto in the middle. both of them leave you with lines of code that only a debug session (in mind or in action) can tell you what lines will be called. I will choose nested code any-day if that means i can simply follow what `if`s are relevant rather than guessing.

beaconstudios 8y ago
I think there are two situations where the return early and nesting if options come into play, and each situation warrants a different strategy: 1. You are guarding for errors or invalid states - you should use early returns or thrown exceptions because these are unexpected states. 2. Your business logic has to branch - you should use a nesting if (or various other options that aren't in scope here, like double dispatch, pattern matching or polymorphism).
erAck 8y ago
Try to "simply follow" when it's nested twelve levels deep. Anyhow, with both approaches, nested or early return, the reader has to read the function from the beginning (or read back until the relevant part) to grasp when exactly a code portion will be executed. In both cases, all if's are relevant, there's no guessing.
- yonixw 8y ago
  when you want to change someone else code, nested blocks lets you know what conditions are needed for each line. when they are gathered at the start you know the condition for the function but for each line you could start a discussion what conditions are necessary and thus make the code atomic (change all by purpose\tests or change none)
  - thecatspaw 8y ago
    you dont reduce nesting by moving all checks to the beginning, you reduce nesting by moving the contents into methods. if (product.hasPrice){ if (someOtherCondition){ Cart cart = cartService.getCartForCurrentUser(); cartService.addToCart(product,1); } } to if(canSellProduct(product)){ addToCurrentCart(product) }
aw1621107 8y ago
> My reasoning is that the reader should know at each line what conditions are assumed. I don’t get it. How do early returns not convey assumptions that can be made in the rest of the function? > Returning in the start is as anti-pattern for me as goto in the middle. both of them leave you with lines of code that only a debug session (in mind or in action) can tell you what lines will be called. This is really confusing to me. What do you mean by “in mind or in action”? In any case, if you have runtime conditions you can’t tell what code will be executed in general just by looking at the code regardless of whether you’re looking at nested code or code with early returns. > I will choose nested code any-day if that means i can simply follow what `if`s are relevant rather than guessing. Are the conditions for early returns not relevant or something? What about them leaves you guessing?
- duncan-donuts 8y ago
  In mind or in action means either your running the logic through your head to debug or you’ve set a break point and you’re actively debugging
- yonixw 8y ago
  they are all gathered at once.the following lines could assume any combinations of those conditions. in simple examples it is not an issue but as the function get more complex I found it excruciating.
  - aw1621107 8y ago
    > the following lines could assume any combinations of those conditions. Do you mind expanding a bit more on this? From what I understand, if you have early returns the only way you can get into the rest of the function is if all of the conditions pass, not any arbitrary combination of them. Also, do you have an example of a more complex function with early functions that you find difficult to read?
  - de_watcher 8y ago
    I just soak everything in asserts. That way there is no need to scroll up for preconditions, be it a nested tree or some returns at the start. And when you move a small piece of code, you're told exactly what you should be thinking about, you need to decide what to do with that assert that represents an assumption.
codingdave 8y ago
And I don't agree with #8, because roles can always be abstracted into, for example, an isAdmin() and then none of the concerns apply. But the article is a good approach to thinking about a multitude of issues. Even if we nitpick or disagree with specific points, the exercise of thinking through it all will certainly result in improved code.
- lsadam0 8y ago
  It sounds like you really just disagree with the example given, not necessarily the rule itself. You're arguing that `isAdmin()` might require a different implementation in a difference scenario, but you have not made a case that the string "admin" should not be constant or enum. I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums. For one, why would you want to re-type the value over and over? And two, it only takes one typo in one manually typed value to introduce a frustrating bug! It's such an easy bug to avoid and costs almost zero to do correctly! Honestly, devs manually typing values that are effectively consts is one of my biggest pet peeves, you're just creating easily avoidable problems for yourself and your team :).
  - codingdave 8y ago
    I'm actually arguing that we should not be focused on nitpicking the details, and instead appreciating the effort taken to think things through, and supporting the idea of using the article as a springboard for our own improvements.
  - kevinconaway 8y ago
    > I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums. My personal opinion is that values should be in constant values if: - Its repeated more than once or referenced outside of the file - The constant value is a "magic number" where the meaning of the value isn't obvious to outsider If the value is used only once and the meaning of the value is clear, I think extracting a constant out is needless indirection. For example: if (flags & 45676 == 0) {} is a magic number that definitely should be a constant, even if its only used once: if (flags & ROLE_ADMIN == 0) {} However something like: pool.setDefaultTimeoutMillis(1000); is perfectly obvious in context IMO
hnzix 8y ago
Let me introduce you to The Magic Number Seven, Plus or Minus Two [0]. This is the amount of state your monkey brain can hold in working memory. This number is why nesting code is almost always heinous. Once the number of nested conditionals exceeds working memory, your monkey brain cannot hold the state and drops all the variables. Deeply nested statements is my number one code smell, and can almost always be easily refactored. [0] https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus...

by yonixw 8y ago

beaconstudios 8y ago

I think there are two situations where the return early and nesting if options come into play, and each situation warrants a different strategy: 1. You are guarding for errors or invalid states - you should use early returns or thrown exceptions because these are unexpected states. 2. Your business logic has to branch - you should use a nesting if (or various other options that aren't in scope here, like double dispatch, pattern matching or polymorphism).

erAck 8y ago

Try to "simply follow" when it's nested twelve levels deep. Anyhow, with both approaches, nested or early return, the reader has to read the function from the beginning (or read back until the relevant part) to grasp when exactly a code portion will be executed. In both cases, all if's are relevant, there's no guessing.

yonixw 8y ago
when you want to change someone else code, nested blocks lets you know what conditions are needed for each line. when they are gathered at the start you know the condition for the function but for each line you could start a discussion what conditions are necessary and thus make the code atomic (change all by purpose\tests or change none)
- thecatspaw 8y ago
  you dont reduce nesting by moving all checks to the beginning, you reduce nesting by moving the contents into methods. if (product.hasPrice){ if (someOtherCondition){ Cart cart = cartService.getCartForCurrentUser(); cartService.addToCart(product,1); } } to if(canSellProduct(product)){ addToCurrentCart(product) }

aw1621107 8y ago

> My reasoning is that the reader should know at each line what conditions are assumed. I don’t get it. How do early returns not convey assumptions that can be made in the rest of the function? > Returning in the start is as anti-pattern for me as goto in the middle. both of them leave you with lines of code that only a debug session (in mind or in action) can tell you what lines will be called. This is really confusing to me. What do you mean by “in mind or in action”? In any case, if you have runtime conditions you can’t tell what code will be executed in general just by looking at the code regardless of whether you’re looking at nested code or code with early returns. > I will choose nested code any-day if that means i can simply follow what `if`s are relevant rather than guessing. Are the conditions for early returns not relevant or something? What about them leaves you guessing?

duncan-donuts 8y ago
In mind or in action means either your running the logic through your head to debug or you’ve set a break point and you’re actively debugging
yonixw 8y ago
they are all gathered at once.the following lines could assume any combinations of those conditions. in simple examples it is not an issue but as the function get more complex I found it excruciating.
- aw1621107 8y ago
  > the following lines could assume any combinations of those conditions. Do you mind expanding a bit more on this? From what I understand, if you have early returns the only way you can get into the rest of the function is if all of the conditions pass, not any arbitrary combination of them. Also, do you have an example of a more complex function with early functions that you find difficult to read?
- de_watcher 8y ago
  I just soak everything in asserts. That way there is no need to scroll up for preconditions, be it a nested tree or some returns at the start. And when you move a small piece of code, you're told exactly what you should be thinking about, you need to decide what to do with that assert that represents an assumption.

codingdave 8y ago

And I don't agree with #8, because roles can always be abstracted into, for example, an isAdmin() and then none of the concerns apply. But the article is a good approach to thinking about a multitude of issues. Even if we nitpick or disagree with specific points, the exercise of thinking through it all will certainly result in improved code.

lsadam0 8y ago
It sounds like you really just disagree with the example given, not necessarily the rule itself. You're arguing that `isAdmin()` might require a different implementation in a difference scenario, but you have not made a case that the string "admin" should not be constant or enum. I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums. For one, why would you want to re-type the value over and over? And two, it only takes one typo in one manually typed value to introduce a frustrating bug! It's such an easy bug to avoid and costs almost zero to do correctly! Honestly, devs manually typing values that are effectively consts is one of my biggest pet peeves, you're just creating easily avoidable problems for yourself and your team :).
- codingdave 8y ago
  I'm actually arguing that we should not be focused on nitpicking the details, and instead appreciating the effort taken to think things through, and supporting the idea of using the article as a springboard for our own improvements.
- kevinconaway 8y ago
  > I honestly cannot think of a reasonable argument for why constant values should not be in static constants or enums. My personal opinion is that values should be in constant values if: - Its repeated more than once or referenced outside of the file - The constant value is a "magic number" where the meaning of the value isn't obvious to outsider If the value is used only once and the meaning of the value is clear, I think extracting a constant out is needless indirection. For example: if (flags & 45676 == 0) {} is a magic number that definitely should be a constant, even if its only used once: if (flags & ROLE_ADMIN == 0) {} However something like: pool.setDefaultTimeoutMillis(1000); is perfectly obvious in context IMO

hnzix 8y ago

Let me introduce you to The Magic Number Seven, Plus or Minus Two [0]. This is the amount of state your monkey brain can hold in working memory. This number is why nesting code is almost always heinous. Once the number of nested conditionals exceeds working memory, your monkey brain cannot hold the state and drops all the variables. Deeply nested statements is my number one code smell, and can almost always be easily refactored. [0] https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus_or_Minus_Two https://en.wikipedia.org/wiki/The_Magical_Number_Seven,_Plus...