dotnet / csharplang

The official repo for the design of the C# programming language
11.4k stars 1.02k forks source link

Proposal/Discussion: Negation of if/while Condition #568

Open HaloFour opened 7 years ago

HaloFour commented 7 years ago

There have been a couple of proposals exploring how improve the readability of conditional operations that involve negation. Some developers find the nesting of parenthesis to be unattractive at best and easy to miss at worst.

I would like to explore the potential of allowing for a negation prefix operator ! to appear outside of the required parenthesis for if and while conditions so that the condition is effectively reversed:

if !(foo is Bar bar)
{
    // stuff
}

while !(finished)
{
    // stuff
}

do
{
    // stuff
}
while !(finished);

Certainly minor and the argument can definitely be made that the negation is even less obvious. It also doesn't address concerns with operators that would still require parenthesis, e.g. is.

lachbaer commented 7 years ago

I give it 👍 because I believe that it improves reading and thinking much.

The downside is that it can be easily overlooked compared to if (!( ... )). Also the position of the ! should be fixed, otherwise there are 3 or more possibilities to write the same expression, leading to confusion and mistakes.

if! (foo is Bar bar) ...
if ! (foo is Bar bar) ...
if !(foo is Bar bar) ...
// worst:
if                          !
  ( foo is Bar bar ) ...

foobar! 😁

Of all I prefer the @HaloFour style. (Hey, good name, "the HaloFour condition syntax" 😇 )

jnm2 commented 7 years ago

I prefer the readability of while (!isFinished) which is already available, and if we had to choose between them, I'd rather have if (foo isnot Bar bar) than if !(foo is Bar bar) because isnot can be used in more places.

lachbaer commented 7 years ago

A little side note: though they are kind of obsolete for this case, an a priori negated condition could call the operator false. if (! TypeWithOperatorTrue) ... gives a compilation error, unless that type has an implicit convertion to boolean as well.

bondsbw commented 7 years ago

Also the position of the ! should be fixed

I disagree, that would prevent no spaces:

if!(foo is Bar bar)

which is less common but is used.

bondsbw commented 7 years ago

@jnm2 agreed with isnot, or perhaps !is or is!. Though it could coexist with this proposal (which can be used for situations other than is).

Both situations would need to consider how pattern variables should work, scoping, etc.

casperOne commented 7 years ago

I'm pretty meh on this. I don't think the readability is any better with the removal of the outer parenthesis (I see it that way, not moving the not operator out).

And if the readability isn't improved, ergo, there's no good reason to do it (IMO).

I think the biggest issue here is that It also opens the door for other things to be included outside the parenthesis (by precedent), which I think will ultimately have a detrimental effect on the language long-term.

CyrusNajmabadi commented 7 years ago

Rather than this, i would just prefer that we relax the grammar for if/while/switch/etc. to not be if ( expr ) but just have it be if expr.

if we did that, then if ! ... just falls out.

Currently the requirement of parens is simply a holdover from C. There is no actual syntactic need/ambiguity it addresses. Which construct this is is already governed by the leading keyword.

--

Another way to think about this is the grammar was always if expr, but there was a normative rule that stated that expr needed to be a parenthesized-expression.

HaloFour commented 7 years ago

@CyrusNajmabadi

Indeed, that is another option. I probably wouldn't prefer that syntax in the normal cases, but it does enable the syntax above as well. Personally I like the parenthesis and I apply them generally anywhere that I might have a non-trivial expression, such as with return statements or ternary conditions.

DavidArno commented 7 years ago

If you championed that, @CyrusNajmabadi, I'd be more than happy to promise to never ever again argue with you or make snide comments about the motives and behaviour of the team.

Deal? 😀

HaloFour commented 7 years ago

itsatrap

CyrusNajmabadi commented 7 years ago

I would do it... if i believed you :D

casperOne commented 7 years ago

Rather than this, i would just prefer that we relax the grammar for if/while/switch/etc. to not be if ( expr ) but just have it be if expr.

Does this mean that one liners such as this:

var cond = true;

if cond Console.WriteLine("true");

Would be legal?

When separating on multiple lines, the syntax looks OK, but when on the same line, it starts to get all jumbled together.

At least that's my perception. This becomes a stylistic issue though, I don't see any real drawback to having this.

Note, I don't see any benefit, but like I said, it's stylistic, and may be of benefit to others (and I acknowledge saving people from having to type millions of parenthesis over their lifetime).

CyrusNajmabadi commented 7 years ago

Would be legal?

Yes

When separating on multiple lines, the syntax looks OK, but when on the same line, it starts to get all jumbled together.

Then don't write it that way. :) It's not like parens would be disallowed. They would just not be required. Similar to how we don't require curlies. It's up to you if you want to mandate them for your own code or not depending on situation.

casperOne commented 7 years ago

Then don't write it that way. :) It's not like parens would be disallowed. They would just not be required. Similar to how we don't require curlies. It's up to you if you want to mandate them for your own code or not depending on situation.

Someone didn't read the rest of my response =P

lachbaer commented 7 years ago

This becomes a stylistic issue though,

Exactly! Like the placing of curly braces or so many other different things. A common style will establish nevertheless, one that is primarily teached in books. But then you should also allow until when everything becomes so 'verbose'.

jnm2 commented 7 years ago
if !foo.Bar() foo.Baz();

Meh.

jnm2 commented 7 years ago

I predict pattern matching complications if this is ever legal.

if obj is Foo foo foo();
jnm2 commented 7 years ago

Knew it wouldn't be long. Ambiguity:

if obj is Foo foo (x).Bar();

if (obj is Foo) foo (x).Bar(); or if (obj is Foo foo) (x).Bar();?

lachbaer commented 7 years ago

Well, very obviously the use of a statement-block must be forced in case the condition parentheses are dropped. Not a big drawback, because most flow statements I see around have braced statement blocks anyway.

Another advantage to me is, that it makes one-line if-statements, e.g. for early returns, optically better readable.

if obj is null { return; }
// vs.
if (obj is null) { return; }

The second is normally spread over 4 lines in total, I guess that that coding style exists, because two clasped regions in one line is "ugly".

With only one braced block however it plays in the same leage as other used constructs.

public Student(string Name, int Age) : base(string Name) { _age = Age; }
string Name { get; private set; }
if foo is null { bar(); }

(First one has other (...), but nevertheless I've seen this construct many times.)

CyrusNajmabadi commented 7 years ago

(First one has other (...), but nevertheless I've seen this construct many times.)

Every if in C# has always had (...), so you've certainly "seen [that] construct many times" as well :)

lachbaer commented 7 years ago

@CyrusNajmabadi ?

CyrusNajmabadi commented 7 years ago

Ambiguity:

C# is rife with ambiguities :) The only rules are:

  1. Old code needs to parse as before (unless the benefit so far outweighs that). For example, we accept that we parse Foo(a < b, c > (d) ) differently that before because the number of people hurt by this is so small, and the new parsing is so worthwhile.
  2. With ambiguities, you have to specify the disambiguation rule.

c# 1.0 shipped with this ambiguity:

(a)-b. But we simply defined how it would be parsed and moved on. We'd do the same here. Likely we'd take the maximal approach to sub production parsing. i.e. since we could, locally, parse out a pattern here, we woudl, leading to the interpretation:

if (obj is Foo foo) (x).Bar();

CyrusNajmabadi commented 7 years ago

@lachbaer You said:

because two clasped regions in one line is "ugly". ... With only one braced block however it plays in the same leage as other used constructs.

But you then referenced a construct that has two clasped regions in one line. You said that that was ok though because you'd seen that construct many times.

But, of course, if it's ok because you've seen it many times, then certainly a multi-clasped 'if' on a single line is ok because you'll have seen (...) with ifs 100% of the time :)

lachbaer commented 7 years ago

@CyrusNajmabadi I thought it was very obvious what I wrote. But especially for you:

lachbaer commented 7 years ago

If removing the need for the condition parantheses, what is a big change for a B/C-derived language since their existence, then IMO the enforcement of a braced statement-block for that new syntax should also be introduced.

CyrusNajmabadi commented 7 years ago

then IMO the enforcement of a braced statement-block for that new syntax should also be introduced.

This seems like one step forward, one step back. :)

lachbaer commented 7 years ago

This seems like one step forward, one step back. :)

I understand what you mean, but sometimes a step back isn't a bad step if you're at the edge of an abyss 😀

I mean, you once wrote that you primarily focus on features for educated programmers. Most advanced code that I have seen use statement-blocks always anyhow (we're talking about control statements here). With this change you had the unique possibility to write history and shift it for the better, for the sake of clarity to all developers.

if obj is Foo foo (x).Bar();

Even if you resolved the amiguities by definition, there is the chance that other developers meant it differently. Why introducing a just possible trap when you can avoid it in the first place? Hasn't C# a leading paradigm of avoiding errors before they occur?

CyrusNajmabadi commented 7 years ago

that's a fair point. But i don't think it's our place to mandate that sort of thing at the language level. i think it would be much more appropriate to just give users style options in the IDE so they can decide on stylistic things themselves. For example, i have to accept that people that want this, will want to write:

if (!(o is string))
    return;

And will want to convert it to:

if !(o is string)
    return;

If we say "if you want to remove hte parens, now you have to write:

if !(o is string)
{
    return;
}

Then that's going to kill the value of this feature for many users.

CyrusNajmabadi commented 7 years ago

Note: i'm not dismissing this concern. It would definitely be a deep one. But i would not consider it a killer for this idea. We would have to consider what we would do here, and it might entail carving out a set of allowed/disallowed things. I'm just trying to clarify that it's unlikely that we could introduce a code-reduction feature, and then tack on a required code-increase feature :D

yaakov-h commented 7 years ago

@lachbaer

then IMO the enforcement of a braced statement-block for that new syntax should also be introduced.

Once again, this is a job for a Roslyn Analyzer. You could make the case to include one with Roslyn/VS/.editorconfig, but other people can have different opinions and switch off the analyzer rule or not include it in the first place.

I think it would be strange to have if expr Thing(); become a valid single line of C#, but after playing around with Swift I'm very much in favour of if expr { block } as regular syntax.

@CyrusNajmabadi Is there any precedent etc. of creating a compiler warning or error from such an ambiguity as the one defined above, rather than quietly parsing it as one or the other?

CyrusNajmabadi commented 7 years ago

Is there any precedent etc. of creating a compiler warning or error from such an ambiguity as the one defined above, rather than quietly parsing it as one or the other?

Nope. There's no precedence afaik. We've always just picked a way to parse. Generics were the most obvious examples of that. It added ambiguities to the language, and we went in the direction of picking one of hte directions to go with (which was not the previous interpretation).

CyrusNajmabadi commented 7 years ago

So, we could go a couple of directions here:

  1. Change the grammar to be: if !_opt ( expr ) EmbeddedStatement. If we do this, there's no ambiguity problem. In effect, all we're doing is allowing an optional !.
  2. Change the grammar to be: if expr EmbeddedStatement, but deal with potential ambiguity fallouts. One way we might address this would be to simply state that "expr" had restrictions or disambiguiation rules attached to it. For example, we might restrict an 'is type t' pattern from being used. it's one of the few expressions that allow for this ending word word form that then ends up potentially causing problems like this.

Right now i have no idea which i prefer :)

yaakov-h commented 7 years ago

'named pattern' being is Foo foo?

CyrusNajmabadi commented 7 years ago

Yes. You caught me mid-edit :)

lachbaer commented 7 years ago

we could introduce a code-reduction feature

I don't see it as such. I rather see it as a clarity feature. As far as I understood the frequently upcoming request for ifnot, if !(..) and alike arises from the wish of a better read- and understandability. And removing the parathesis (code-reduction) is just a possibility to achieve this goal.

Incidentially @CyrusNajmabadi as a team member came up with this idea. I guess nobody had ever believed that a possible request for removing a classical C-construct would be considered as a solution 😇

if !(o is string)
    return;

I believe the benefits of forcing statement-blocks to prevent devious expression constructs outweigh the possible cases where somebody wants to use that style above!

By introducing "clampless-conditions" with statement-blocks-only there will exist a further way to express something. One that is syntactically completely new to the C-style. Why introducing another one - with single substatements - when you can already express the same so many different ways? And what are the odds that the above style is allowed by guide lines, but if !(o is string) \n { return; } hasn't the same value?

Also, there are other constructs where braces are mandatory. Leaving expression bodies aside every method must have braces. try and catch need them for a reason, though they could be nested without 'em when just defining a hierarchy (e.g. finally breaks the inner nest). Llambdas that start with a control statement need the braces (int i) => { switch (i) { case 1: return 2; default: return 3; }.

And as stated above, the vast majority of advanced programmers are probably already using braces anyway. The rest can keep using the old constructs or convert. The benefits of clarity outweigh traditions used by a few.

However, I may completely be mistaken by the number of (min. average) developers who are not using braces in their production code! 🖖

Once again, this is a job for a Roslyn Analyzer. You could make the case to include one with Roslyn/VS/.editorconfig, but other people can have different opinions and switch off the analyzer rule or not include it in the first place.

Absolutely agree. You already have answered to your comment yourself 😉 Nevertheless my doubts for allowing single blockless statements still last.

lachbaer commented 7 years ago

Btw, when talking about uncluttering conditions, has anyone an idea on how !(o is string) can be expressed better without introducing isnot? Maybe o is! string, in light accordance with !=?

if obj is! string { return; }    // to be read as "isn't"
DavidArno commented 7 years ago

My vote is definitely for "Change the grammar to be: if expr EmbeddedStatement, but deal with potential ambiguity fallouts."

In reality, whilst we can all dream up ugly ambiguities, sure most devs want to be able to read their own code and so will avoid writing such ugly code?

The following seems nice and readable to me, so I'd use the optional parentheses feature here:

if anyTestFailed return;

The following isn't readable, so I wouldn't write it that way:

if
(
!!!!                       anyTestFailed
){

                                              return;
                                                        }

I can make ugly code with existing rules and ambiguities already exist. So the fact that folk can make ugly code from this proposal probably shouldn't be a reason to dismiss it.

yaakov-h commented 7 years ago

@lachbaer: How about if obj.IsNotOfType<string>() { return; }?

public static bool IsNotOfType<T>(this T obj) => !(obj is T);
lachbaer commented 7 years ago

@yaakov-h !(obj is type) is a too common expression to have a wrapper method for it, imo.

DavidArno commented 7 years ago

Btw, when talking about uncluttering conditions, has anyone an idea on how !(o is string) can be expressed better without introducing isnot?

Again, to my mind:

if !o is string return;

does the job just fine.

yaakov-h commented 7 years ago

@lachbaer I haven't seen it in many places, myself.

@DavidArno What if o is a bool?

yaakov-h commented 7 years ago

Actually, I think for negative type checking / negative pattern matching that #138 would work quite well:

void Foo(object o)
{
    guard (o is Bar b) else return;

    // Do something with `b`...
}
lachbaer commented 7 years ago

if !o is string return;

See specifications chapter 7.3.1 "operator precedence". is has less precedence than !. This one shouldn't work the way you intend it to.

lachbaer commented 7 years ago

@DavidArno I just fear that especially less experienced programmers will write expressions like

if obj is Foo foo(x).Bar();

and produce errors or get confused why their expression does not compile. Nevertheless I get your point that an experienced programmer would care for ambiguities already at coding time, as he does with parentheses when he doesn't know the precedence table by heart.

lachbaer commented 7 years ago

@DavidArno

if (!value1 is double) return;
//Error CS0023  Operator '!' cannot be applied to operand of type 'double'  
DavidArno commented 7 years ago

@lachbaer, good point. That'll be why I can write the following today and end with False being printed:

var x = true;
if (!x is bool y) ; 
Console.WriteLine(y);

But really, who writes ugly ambiguous code like this in real life 😉

lachbaer commented 7 years ago

@DavidArno Alone the fact that you came up with if !o is string return; shows the source of potential errors. And you are an experienced developer. That's all I'm ever talking about when I state the term human factors over and over 😀

DavidArno commented 7 years ago

@lachbaer, Given that the same problem occurs with:

if (!o is string) return;

then no new potential error is being introduced.

lachbaer commented 7 years ago

@DavidArno No, just in principle 😃

jnm2 commented 7 years ago

I don't see why you'd go to the immense trouble of removing parens if another solution which is more widely useful is an isnot keyword.

if (x isnot Foo) return;