5 ms·

> When trying to define default values, people get confused with expressions like a === b ?? c and assume it will be parsed as a === (b ?? c). When in actuality

by kmoser 3y ago

> When trying to define default values, people get confused with expressions like a === b ?? c and assume it will be parsed as a === (b ?? c). When in actuality it will be parsed as (a === b) ?? c. I'll never understand why programmers don't simply put parens where they want the expression to be evaluated, rather than relying on their (sometimes incorrect) assumption about operator precedence. I want to chalk this up to hubris, but it's probably just laziness.

SenHeng 3y ago
Prettier oftentimes removes a lot of parentheses that I had hoped it would leave. I’ve taken naming them as variables instead. Definitely more verbose but more readability. const aIsB = a === b; … aIsB ?? c …
- deleted 3y ago
  [deleted]
- pests 3y ago
  That is how it evaluates, not what the original author intended in the example in the OP. return a === b ?? c; The intent was const bOrDefault = b ?? c; return a === bOrDefault; You posted the actual (buggy) evaluation. I agree that its better if that is what you intend.
- pests 3y ago
  Wait, your example doesn't even make sense to me on second thought (and was the ultimate point in the article) aIsB is guaranteed to not be null or undefined so the ?? is a no-op. You just did a comparison so it's either true or false. For general clarify for precedence I still agree but in this case it's a silly thing to do. It would be more clear without the syntax shorthand: const ret = a === b if(ret == null || ret == undefined) { ret = c } return ret
  - SenHeng 3y ago
    I was referring particularly to my parent's point about adding parentheses to remove ambiguity. That prettier oftentimes removes parentheses that I explicitly want. So rather than risk that, I remove the ambiguity by moving one of the comparisons off into a named variable.
    - pests 3y ago
      I agree. I was just pointing out that this is a case prettier wouldn't do anything. The expression parses differently and adding or removing parentheses to any subexpression will change its meaning. That by manually doing the transform in your example that you would realize your cody was a no-op or that the intended version was what I posted.
robinson7d 3y ago
In fact, I’ve seen an extension several times: senior developers performing code review, and requesting that unnecessary, but clarifying, parens be removed. Not excessive parentheses, a single pair in a line or statement.
captbaritone 3y ago
Author of the rule and blog post here. I agree that, for me, I appreciate the extra clarity of explicit parens. This lead me to explore a VSCode plugin which visually show the implicit parens even if they are not present in the code: https://jordaneldredge.com/blog/a-vs-code-extension-to-combat-js-precedence-confusion/ https://jordaneldredge.com/blog/a-vs-code-extension-to-comba... I like the idea that different readers of the same code base could opt for differing levels of explicitness when it comes to operator precedence. One thing that working on the project helped demonstrate for me is that adding parens around _every_ subexpression is _way_ too noisy. So, you need to draw the line somewhere. But for me, I prefer drawing that line on the noisier side.
- kmoser 3y ago
  > One thing that working on the project helped demonstrate for me is that adding parens around _every_ subexpression is _way_ too noisy. But for me, I prefer drawing that line on the noisier side. I prefer the noisier side as well. When adding parens to indicate/force precedence, I will sometimes split my expressions into multiple lines, with indentation, as an aid to legibility, e.g. instead of ( ( a + b ) / ( ( c - d ) / e ) ) I might write: ( ( a + b ) / ( ( c - d ) / e ) ) I've found this not only quite legible, but also fairly easy to edit because you can easily match parens visually.
  - kawhah 3y ago
    Thanks, I hate it.
  - FlyingAvatar 3y ago
    I appreciate the attempt, but don't the explicit parens make it pretty clear? I feel like if you just remove the spaces around the parens themselves, it's the most clear: ((a + b) / ((c - d) / e)) Nowhere I have worked would pass a code review for what you have proposed.
    - innocenat 3y ago
      If it isn't just a b c d e but longer variable name or function call, then I imagine it wouldn't be as clear.
viraptor 3y ago
Because there's a threshold of where you know how things will be parsed. You wouldn't write: a=(b+c) Because you know it's not necessary. But we're human and sometimes make mistakes when we're sure we didn't. That's also why we allow for some misunderstanding, (rather than (writing (using) (sentence trees))).
- kmoser 3y ago
  Agreed, that would be one level too far, and thus unnecessary. I'm not arguing that every operator should be parenthesized, but I think parens should be used more than most programmers tend to do. On a related note, I occasionally write expressions that both assign and test, e.g. if ( ( a = b ) == c ) and in those cases I parenthesize liberally and always write a comment that indicates yes, I intend to make an assignment in the middle of a test for equality.
  - demondemidi 3y ago
    Oof really? That’s a horrible pattern! In my professional opinion of course. Separation of causes, man! :)
    - porridgeraisin 3y ago
      It's very common in C while((buf = resultofstreamingoperation() != null) { } The ` != null ` is redundant here but you can imagine where it'd be required.
      - sweetjuly 3y ago
        A more common form (and slightly less evil) is for error checking: if ((error = do_a_thing())) { // some error happened, good thing we saved the error code } Given the standard "zero is success" paradigm, handling errors is very succinct.
      - demondemidi 3y ago
        You are arguing "common" == "good". That's not true.
GeneralMaximus 3y ago
I personally make liberal use of parens to make the precedence of operators as clear as possible. Then I run formatting tools like Prettier or rustfmt on my code, which remove any parens that are not strictly required. This way, I end up with an expression that is clear as well as concise. Best of both worlds.
- captbaritone 3y ago
  This is the approach I take as well. But it's not necessarily best of both worlds. Removing (or not inserting) technically useless parens can obscure bugs if you are not perfectly fluent in the precedence rules of the language. Their doubly easy to miss if there's a lot going on on one line. Here's one example that the rule caught: https://github.com/captbaritone/vscode/blob/ab86e0229d6b4d0cb49cfd6747c92cafcd2bd4af/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts#L456:L456 https://github.com/captbaritone/vscode/blob/ab86e0229d6b4d0c... I've been writing JS for over 10 years now, and I'm not sure I would have caught that in code review.
  - amatecha 3y ago
    Ahh... there's a strict equality operator comparing undefined (both a value and a type) to something that's being coerced to a boolean... it will always return false. The first thing I look at when I see a strict equality comparison is "are the operands actually the same type" because that's a very common mistake (or so I've noticed)! Huh, now I'm curious if my IDE would highlight this with a warning... will have to check!
demondemidi 3y ago
I think one reason is that when I am refactoring I am thinking about other larger logic, and as expressions get reduced, I become blind to the details that I and previously internalized as correct. Linters really help because I can’t always be thinking at both ends of the abstraction.
runeks 3y ago
> I want to chalk this up to hubris, but it's probably just laziness. You may be able to chalk it up to some “senior” reviewer who asks a junior developer to remove the parens because “they aren’t needed”, and the junior developer doesn’t know how to argue their case.
matsemann 3y ago
I put them there, but some linter will then complain about "unnecessary parenthesis", so you can't win.
billpg 3y ago
LISP has entered the chat.
hoosieree 3y ago
Precedence was a mistake and new language designs should throw it in the garbage. Left to right is fine. So is right to left. Just pick one and stick with it. “But PEMDAS!” Ok, YOU can use parens.

by kmoser 3y ago

SenHeng 3y ago

Prettier oftentimes removes a lot of parentheses that I had hoped it would leave. I’ve taken naming them as variables instead. Definitely more verbose but more readability. const aIsB = a === b; … aIsB ?? c …

deleted 3y ago
[deleted]
pests 3y ago
That is how it evaluates, not what the original author intended in the example in the OP. return a === b ?? c; The intent was const bOrDefault = b ?? c; return a === bOrDefault; You posted the actual (buggy) evaluation. I agree that its better if that is what you intend.
pests 3y ago
Wait, your example doesn't even make sense to me on second thought (and was the ultimate point in the article) aIsB is guaranteed to not be null or undefined so the ?? is a no-op. You just did a comparison so it's either true or false. For general clarify for precedence I still agree but in this case it's a silly thing to do. It would be more clear without the syntax shorthand: const ret = a === b if(ret == null || ret == undefined) { ret = c } return ret
- SenHeng 3y ago
  I was referring particularly to my parent's point about adding parentheses to remove ambiguity. That prettier oftentimes removes parentheses that I explicitly want. So rather than risk that, I remove the ambiguity by moving one of the comparisons off into a named variable.
  - pests 3y ago
    I agree. I was just pointing out that this is a case prettier wouldn't do anything. The expression parses differently and adding or removing parentheses to any subexpression will change its meaning. That by manually doing the transform in your example that you would realize your cody was a no-op or that the intended version was what I posted.

robinson7d 3y ago

In fact, I’ve seen an extension several times: senior developers performing code review, and requesting that unnecessary, but clarifying, parens be removed. Not excessive parentheses, a single pair in a line or statement.

captbaritone 3y ago

Author of the rule and blog post here. I agree that, for me, I appreciate the extra clarity of explicit parens. This lead me to explore a VSCode plugin which visually show the implicit parens even if they are not present in the code: https://jordaneldredge.com/blog/a-vs-code-extension-to-combat-js-precedence-confusion/ https://jordaneldredge.com/blog/a-vs-code-extension-to-comba... I like the idea that different readers of the same code base could opt for differing levels of explicitness when it comes to operator precedence. One thing that working on the project helped demonstrate for me is that adding parens around _every_ subexpression is _way_ too noisy. So, you need to draw the line somewhere. But for me, I prefer drawing that line on the noisier side.

kmoser 3y ago
> One thing that working on the project helped demonstrate for me is that adding parens around _every_ subexpression is _way_ too noisy. But for me, I prefer drawing that line on the noisier side. I prefer the noisier side as well. When adding parens to indicate/force precedence, I will sometimes split my expressions into multiple lines, with indentation, as an aid to legibility, e.g. instead of ( ( a + b ) / ( ( c - d ) / e ) ) I might write: ( ( a + b ) / ( ( c - d ) / e ) ) I've found this not only quite legible, but also fairly easy to edit because you can easily match parens visually.
- kawhah 3y ago
  Thanks, I hate it.
- FlyingAvatar 3y ago
  I appreciate the attempt, but don't the explicit parens make it pretty clear? I feel like if you just remove the spaces around the parens themselves, it's the most clear: ((a + b) / ((c - d) / e)) Nowhere I have worked would pass a code review for what you have proposed.
  - innocenat 3y ago
    If it isn't just a b c d e but longer variable name or function call, then I imagine it wouldn't be as clear.

viraptor 3y ago

Because there's a threshold of where you know how things will be parsed. You wouldn't write: a=(b+c) Because you know it's not necessary. But we're human and sometimes make mistakes when we're sure we didn't. That's also why we allow for some misunderstanding, (rather than (writing (using) (sentence trees))).

kmoser 3y ago
Agreed, that would be one level too far, and thus unnecessary. I'm not arguing that every operator should be parenthesized, but I think parens should be used more than most programmers tend to do. On a related note, I occasionally write expressions that both assign and test, e.g. if ( ( a = b ) == c ) and in those cases I parenthesize liberally and always write a comment that indicates yes, I intend to make an assignment in the middle of a test for equality.
- demondemidi 3y ago
  Oof really? That’s a horrible pattern! In my professional opinion of course. Separation of causes, man! :)
  - porridgeraisin 3y ago
    It's very common in C while((buf = resultofstreamingoperation() != null) { } The ` != null ` is redundant here but you can imagine where it'd be required.
    - sweetjuly 3y ago
      A more common form (and slightly less evil) is for error checking: if ((error = do_a_thing())) { // some error happened, good thing we saved the error code } Given the standard "zero is success" paradigm, handling errors is very succinct.
    - demondemidi 3y ago
      You are arguing "common" == "good". That's not true.

GeneralMaximus 3y ago

I personally make liberal use of parens to make the precedence of operators as clear as possible. Then I run formatting tools like Prettier or rustfmt on my code, which remove any parens that are not strictly required. This way, I end up with an expression that is clear as well as concise. Best of both worlds.

captbaritone 3y ago
This is the approach I take as well. But it's not necessarily best of both worlds. Removing (or not inserting) technically useless parens can obscure bugs if you are not perfectly fluent in the precedence rules of the language. Their doubly easy to miss if there's a lot going on on one line. Here's one example that the rule caught: https://github.com/captbaritone/vscode/blob/ab86e0229d6b4d0cb49cfd6747c92cafcd2bd4af/src/vs/workbench/contrib/terminal/browser/terminalProfileResolverService.ts#L456:L456 https://github.com/captbaritone/vscode/blob/ab86e0229d6b4d0c... I've been writing JS for over 10 years now, and I'm not sure I would have caught that in code review.
- amatecha 3y ago
  Ahh... there's a strict equality operator comparing undefined (both a value and a type) to something that's being coerced to a boolean... it will always return false. The first thing I look at when I see a strict equality comparison is "are the operands actually the same type" because that's a very common mistake (or so I've noticed)! Huh, now I'm curious if my IDE would highlight this with a warning... will have to check!

demondemidi 3y ago

I think one reason is that when I am refactoring I am thinking about other larger logic, and as expressions get reduced, I become blind to the details that I and previously internalized as correct. Linters really help because I can’t always be thinking at both ends of the abstraction.

runeks 3y ago

> I want to chalk this up to hubris, but it's probably just laziness. You may be able to chalk it up to some “senior” reviewer who asks a junior developer to remove the parens because “they aren’t needed”, and the junior developer doesn’t know how to argue their case.

matsemann 3y ago

I put them there, but some linter will then complain about "unnecessary parenthesis", so you can't win.

billpg 3y ago

LISP has entered the chat.

hoosieree 3y ago

Precedence was a mistake and new language designs should throw it in the garbage. Left to right is fine. So is right to left. Just pick one and stick with it. “But PEMDAS!” Ok, YOU can use parens.