Stylecopanalyzers: Rule proposal: Readability rule - Operator placement for wrapped expressions

Created on 27 Apr 2015  路  11Comments  路  Source: DotNetAnalyzers/StyleCopAnalyzers

(EOL = End of Line, BOL = Beginning of Line)

I think StyleCop should dictate (or at least try to dictate) the position of operators. An example:

this is on one line, and it's ok...

if (true && false) 
{ 
} 

But on two lines?

if (true && 
false) 

or

if (true 
&& false) 

the same for +, -, *, /

And then there are ? (with :) and ??.

I think that these operators should be put at End of Line, so that if the second operand is a brace the line begins with the brace.

last but not least is the "." operator. Here I think it's easier. It should go on the new line for the same reason as the "this" that is necessary. When you press "." the vs shows you the list of the properties/methods/fields of the object.

So it should be:

var temp3 = new int[5]
.Cast<long>()
.Cast<int>();

Originally posted by xanatos http://stylecop.codeplex.com/workitem/6791

category-readability needs discussion new rule proposal

All 11 comments

I agree there should be rule regarding this. But in my opinion operator should be placed always at the beginning of line.

  • First, it is easier to see that this is continuation from previous line.
  • Second, it allows to easy comment out part of expression - no dangling operator at the end of the line preceding commented out line.
  • Third, it better suits mental model (at least mine) - I think "If something and_something_else" not "if something_and something_else". You get my point here

I agree with placement at the beginning of the line.

I'm not sure how we would formally define this rule and present it to users. Perhaps "Binary operators must not be followed by a newline" or "Wrap binary expressions before the operator"?

I am not native English, so I don't feel competent enough regarding exact wording :)

Would it be possible to separate the rule for ? and ?? from the other operators? I'm with the original poster on this one, as visually I parse:

someCondition ?
  resultIfTrue :
  resultIfFalse

In the same manner as:

if (someCondition)
{
    resultIfTrue
}
else
{
    resultIfFalse
}

It's the level of indentation that most clearly delimits the blocks, whilst if we have:

someCondition
  ? resultIfTrue
  : resultIfFalse

I lose both the ability to immediately determine that someCondition is a boolean test (without my eye skipping to the next line) and the ability to clearly pick out resultIfTrue and resultIfFalse.

I know even amongst my team we're divided on this, so if rules were available to enforce both behaviours, with one enabled by default but the ability to adopt the other if we see fit, that would be fine.

Normally, I like the break with the dot operator on the start of the next line.

string foo = Bar
    .Baz
    .Blech;

However, the new null conditional operator is kind of a special case. This looks wrong:

string foo = Bar
    ?.Baz
    ?.Blech;

But I think this looks wrong, too, especially if you break before the dot otherwise:

string foo = Bar?.
    Baz?.
    Blech;

Apparently this is legal and I guess is ok, but the ?. is actually one operator and it now has a line break in the middle of it, so makes me queasy, too:

string foo = Bar?
    .Baz?
    .Blech;

Thoughts on breaking null conditional operators?

Why does

int foo = Bar
    ?.Baz
    ?.Blech;

look wrong to you?
It immediately gives you proper info that Baz and Blech are called conditionally. No need to scan end of previous line to see this.

I guess I could get used to it. Is this the consensus of how it should be done? I kind of feel that the ? belongs to the Bar, not the Baz.

@GregReddick I agree that I mentally think of the ? belonging to Bar, not Baz. But at the same time, when quickly glancing over code, it's a whole lot easier to see that ?.Bar is a null-check than if it's hidden at the end of the previous line.

I like the ?.Bar syntax for that reason, and because it's consistent with how I typically start lines with . already.

Okay. Having played with it, I agree that ?.Baz is the best choice of the three. It makes it consistent that operators should appear at the beginning of the line.

I prefer

a = someCondition
  ? resultIfTrue
  : resultIfFalse

to

a = someCondition ?
    resultIfTrue :
    resultIfFalse

And

a = b
    + c

over

a = b +
    c

And

if (a
   && b)
{
}

over

if (a &&
    b)
{
}

The operator at the beginning of the line (plus the indenting) makes it easy to see that the line is a continuation.

I completely agree about all except the last two. I personally prefer

a = b +
      c

and

if (a &&
    b &&
    c)

myself, but it's just a personal preference. In these cases, I like the variables lined up vertically. I'm sure I could just get used to the change, though, if we decided that always starting a line with an operator was the way to go.

Should this be split into two rules? One dealing with .Baz and ?.Baz, while another deals with things like +-*/ and &&?

In our team we ran in to this issue due to assignment operator. Half of the legacy code has it placed on the next line like so:

var myVariable
    = "Hello world";

We would definitely like to invest in StyleCop to have a rule (+auto-fix for this), so that we can quickly clean in up. Was there ever a consensus on a particular set of operators and how majority of people would want them to be formatted?

From what I understand current preference is:

fooBar
   .Method()
   .SomeOtherMethod();

fooBar
    ?.Method()
    ?.SomeOtherMethod();

var myVariable =
    "Hello world";

Maybe we can start with this set and expand it later, once/if we reach an agreement?

As to the name, since we want some operators on the previous line and some on the next - I think we have two options:
1) Add one rule Binary operators should be placed correctly (or remove binary, as Elvis, aka ?:, is not binary and we may include it in the future)
2) Add two rules:
- one for Binary operators should be placed on the line with first operand
- two for Binary operators should be placed on the line with second operand

My vote is for Operators should be placed correctly.


I understand people may have different preferences, but I thinking allowing people to choose how it should be formatted is defeating the purpose of common coding styles. Yeah, it works if you only care about your project, but if you work on multiple projects it doesn't help much with readability. Another problem is when you want to integrate two different code-bases together and they use different styles.

In my opinion, we want coding styles to be attached to the language, rather than the project.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

SurajGupta picture SurajGupta  路  5Comments

arashk1368 picture arashk1368  路  4Comments

drewnoakes picture drewnoakes  路  3Comments

patriksvensson picture patriksvensson  路  6Comments

mzhukovs picture mzhukovs  路  4Comments