Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flexibility of operator spacing / mixing of precedence #160

Open
MichaelChirico opened this issue Oct 27, 2020 · 5 comments
Open

flexibility of operator spacing / mixing of precedence #160

MichaelChirico opened this issue Oct 27, 2020 · 5 comments

Comments

@MichaelChirico
Copy link
Collaborator

I'm not sure how to capture the issue precisely, but I think the current wording of the style guide around operator spacing can lead to some hard-to-read code:

a_constant + 1 / 12

is IMO pretty hard to read vs

a_constant + 1/12

I think what's going on here is + is lower precedence than /, but they have equal spacing. Removing the spacing around / is a visual cue about operator precedence.

The logic breaks down when we use more complicated expressions with more than two levels of operator precedence:

addend  +  base^exponent / denominator # ??

Perhaps the recommendation in this case could be to use parentheses to limit any sub-expression to at most 2 levels of operator precedence. Maybe overkill.

In any case, I think a simple case where + | - and * | / are mixed could be spelled out pretty easily.

@hadley
Copy link
Member

hadley commented Aug 23, 2021

Or if you wanted to make it more specific, it could just be "writing fractions without spaces is also ok".

@hadley
Copy link
Member

hadley commented Aug 23, 2021

I'm pretty sure I prefer (a + b) / (c + d) to (a + b)/(c + d), so maybe this only applies to simple fractions?

@luciorq
Copy link

luciorq commented Aug 24, 2021

a_constant + 1 / 12

is IMO pretty hard to read vs

a_constant + 1/12

I would argue that it is only harder to read for someone already used to writing in that way.

Spaces generally enhance readability.

The specific topic on numeric fractions (1/12 vs. 1 / 12) is just arithmetic thinking bias, treating fractions as a single entity and not an operation.

@MichaelChirico
Copy link
Collaborator Author

I'm pretty sure I prefer (a + b) / (c + d) to (a + b)/(c + d), so maybe this only applies to simple fractions?

I agree with your preference, and it's consistent with my point I think.

There, ( is the high-precedence operator, so the spacing around lower-precedence / is good.

I would argue that it is only harder to read for someone already used to writing in that way.

Spaces generally enhance readability.

The specific topic on numeric fractions (1/12 vs. 1 / 12) is just arithmetic thinking bias, treating fractions as a single entity and not an operation.

I think 1 / 12 is fine by itself, my point in this issue is about mixed precedence.

I think most readers know intuitively that / is higher precedence than +, but the spacing as a nudge is I believe helpful:

x + 1 / 2
# vs
x + 1/2

It's clearer that / operates first. I think it's basically the same as why we allow ^ without spaces:

x + 3 ^ 2
# vs
x + 3^2
## or
x / 3 ^ 2
# vs
x / 3^2

I think the issue is even clearer in expressions with more operators:

a * b + c * d - e * f / g
# vs
a*b + c*d - e*f/g

Since the Google C++ guide was referenced elsewhere, I'll note that's in line with their recommendation:

https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace

@hadley
Copy link
Member

hadley commented Sep 1, 2021

I agree with your preference, and it's consistent with my point I think.
There, ( is the high-precedence operator, so the spacing around lower-precedence / is good.

If I carefully think about it, I can understand your point about high vs low precedence operators, but it's something I'd need to think about every time, so we'd need to figure out a simpler way of framing it in order to include in the style guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants