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

Discretional use of parenthesis nullifies omitting unnecessary parenthesis #278

Closed
samkim102 opened this issue Jun 14, 2017 · 6 comments
Closed

Comments

@samkim102
Copy link

We recently decided to adopt this style guide, but the following statement caused us some headache:

In larger expressions, optional parentheses can sometimes make code read more clearly.

To be precise, larger expression is very subjective and a developer who is more accustomed to having parenthesis around conditional statements will insist the use of parenthesis will enhance readability. As a minimalist, I would argue against any use of parenthesis if it does not alter the meaning of the conditional statement.

Any thoughts?

@designatednerd
Copy link
Member

A lot of the places I've see them be helpful is clarifying where an equality check is being used as a single boolean:

let thing = thingA == thingB
// vs. 
let thing = (thingA == thingB)

Order of operations there makes it clear that thing is a boolean which will have the value of "is thingA equal to thingB, but that takes a lot longer to parse visually with the first option than the second.

This is also helpful with compound statements, though sometimes they're necessary to clarify order of operations as well:

if thingC && (thingD || thingE) { ...

Basically, it's mostly about scannability: if it takes you even a second longer to parse as you read through it, that gets old real quick. And if it takes junior developers way longer to reason about what's happening, it can open the door to disaster.

@moayes
Copy link
Member

moayes commented Jun 14, 2017

I agree with @designatednerd. Personally, I also prefer using parentheses around expressions. I'm also a minimalist but I believe that code should be readable and clear for people; otherwise, the compiler doesn't really care. The way I look at it is that I expect to see a simple assignment after =, e.g. let thing = foo. A parenthesis after that, though, is a visual cue that the assignment happens somewhere later, probably at the end of the line:

let thing = ( /* some short or long logic here, skip it */ ) ? foo : bar

@rcritz
Copy link
Contributor

rcritz commented Oct 25, 2018

@samkim102 if I read your question correctly, you're not asking about the cases that the other comments addressed but, instead, are asking about the use of parens around the condition of an if statement.

If that's the case, please help me understand how to more clearly state that parens should not be used in that case. The current wording of the guide seems unequivocal to me.

@rayfix
Copy link
Contributor

rayfix commented Nov 15, 2018

Observation: The rule could be changed to say to use parenthesis only when it promotes clarity. I believe the original guideline was put there to prevent:

// go for a Sunday drive
if (day.isSunday) {
   vehicle.drive()
}

On the other hand, if you are eliding parenthesis because you know the precedence of the other operators involved, you are probably being too clever for your own good.

@rcritz
Copy link
Contributor

rcritz commented Nov 18, 2018

The problem, as I understand it, is that some folks seem to think parens are required for clarity in all cases. Clearly, that's not true in the vast majority of cases in if and switch statements. Usage in assignment statements, as other commenters have mentioned, is a murkier swamp. "Promotes clarity" is a subjective criteria, which doesn't seem to help the situation here.

If I don't hear more from the original author in the next week, I'm going to close this for lack of activity.

@rcritz
Copy link
Contributor

rcritz commented Jan 31, 2019

Closed for lack of response.

@rcritz rcritz closed this as completed Jan 31, 2019
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

5 participants