dotnet / csharplang

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

Champion "Negated-condition if statement" #882

Open gafter opened 6 years ago

gafter commented 6 years ago

See https://github.com/dotnet/csharplang/issues/568 where this idea was recently introduced.

Neme12 commented 6 years ago

@HaloFour

Yes I realize this is already possible with booleans but I never thought of the !true as a negative pattern and I see these as fundamentally different operators.

In my way of thinking this is more like a coincidence, that the negation of true is the only value that is not true, whereas there is a lot of values that are not 5. On the other hand, !5 makes it seem like it's possible to negate 5 (like you can in C++ and get 0) rather than it being a pattern. It's like the difference between x == !true and x != true.

And maybe it is a little silly that in this case you could do the same thing in 2 ways, but hey - we already have x is 5 and x == 5 and I would argue that this is a much smaller issue/curiosity than even that because this only affects bool.

Joe4evr commented 6 years ago

we already have x is 5 and x == 5

I'd say that while these look similar and behave the same, when the compiler breaks them down syntactically, they're actually quite different.

The idea is that many different kinds of patterns could can be used instead of a constant pattern, but patterns can't be used with ==.

Starwer commented 6 years ago

Apparently, the if(); else from @alrz made a good laugh... But I think he has a point: what about allowing the following syntax ?

if (b1 is Type obj) else 
{
    throw new ArgumentException();
}

(Please note: no semicolon)

Even though it sounds awkward if you speak it out, it is unambiguous on what should be expected and enable the Formatter/IntelliSense to sort that out that obj is useless and should issue a warning.

AustinBryan commented 6 years ago

So in the original post, someone suggested doing if expr, instead of if (expr) which would automatically makeif !(expr)` valid.

Is the champion for just if !(expr) or if expr in general?

iam3yal commented 6 years ago

@AustinBryan Only if !(expr).

AustinBryan commented 6 years ago

@eyalsk Fair. I think if !(expr) easier to read than if (!expr) and especially if (!(expr)) which is commonly used

CyrusNajmabadi commented 5 years ago

@gafter I'm working on an implementation of this proposal here: https://github.com/dotnet/roslyn/pull/30945 in the features/guard-statements branch.

gafter commented 5 years ago

@CyrusNajmabadi : @AlekseyTs is the feature champion. As such it is his responsibility to shepherd this feature into C# 8.0. I don't know what his current plans are.

CyrusNajmabadi commented 5 years ago

@gafter Thanks! @AlekseyTs i'd be happy to work with you on this. I have an implementation approach that i think is viable over at: https://github.com/dotnet/roslyn/pull/30945. It tries to strike a balance between being correct wrt to the language change, while also minimizing the need for lots of downstream fallout from the language feature. My hope is that strategy can help lower costs (for both our own implementation, and the impact on the ecosystem) to help make it easier for this to be included in a release sooner. Let me know what you think!

gafter commented 5 years ago

At LDM today we noted that the proposed addition of a negated pattern might make this feature less compelling. For example, you'd be able to write

    if (o is not null) ...
Thaina commented 5 years ago

@gafter Is it possible to have that negated pattern shorthanded as just not ?

    if (o not null) // contextly obvious to be a pattern
gafter commented 5 years ago

@Thaina You mean something like an expression that is

expression not pattern

Sure, that would be possible, but we'd still need the negated pattern forms for switch, which means they would work for is as well, so this suggested expression form would be quite redundant.

CyrusNajmabadi commented 5 years ago

Sure, that would be possible, but we'd still need the negated pattern forms for switch, which means they would work for is as well, so this suggested expression form would be quite redundant.

@gafter I think i would add more of a rule like this:

expr_at_some_precedence_level:
     expr not_pattern

not_pattern:
    not pattern

Basically, if there are any 'unary_patterns', then they could be legal directly after expr without needing the is (as long as they didn't cause ambiguity).

gafter commented 5 years ago

@CyrusNajmabadi Right, but you could also write

e is not null

making the proposed expression form quite redundant.

CyrusNajmabadi commented 5 years ago

@gafter i definitely get that :) The originating question there was:

@gafter Is it possible to have that negated pattern shorthanded as just not ?

Basically, the point would be just to have the shorthand, even if it was a redundant form. i.e. will it be so common to write x is not ... that it would just be nicer to allow x not .... I'm personally on hte fence. It's clean and consistent to not allow and to force 'is'. But, at the same time, it seems like it could be common enough to be a nice to have. I don't really care enough to push, but perhaps a read of the LDM would be in order on this?

gafter commented 5 years ago

@CyrusNajmabadi I have to say it just reads wrong to me. true is not false is an obviously true sentence, but true not false does not even have a verb.

marksmeltzer commented 5 years ago

@gafter

true isnt false?

I don't think that pattern mixes well with is though: if (a is isnt null). The primary pattern reads well enough though: if (a isnt null).

Really, this doesn't come down to rules of English though... This syntax all becomes pure utilitarian convention once we type it 10,000 times. I would be perfectly happen with if (a not null), etc. Using not as an operation seems like a useful contraction in specific cases like null comparison.

In languages like SQL, not is most commonly as an adverb though, as you suggest. For example, in SQL's not in, the not is a modifier to the in operation.

I would vote for not null being a specifically recognized case. Personally, I would not want to see not generalized in the way that @CyrusNajmabadi described above...

jnm2 commented 5 years ago

I really like x is not null.

Thaina commented 5 years ago

Well, maybe I just don't like the double separate keyword

o is not true might be already good as is. And if we really want to we could just have isnt being a shorthand macro that will be transpiled back to is not

YairHalberstadt commented 5 years ago

@Thaina C# has no concept of macros, and I don't think this feature would be worth introducing them for...

dbmercer commented 5 years ago

Every new syntactic feature we add to the language adds to the cognitive load of a new programmer. It is easy for us, who are experts in C#, to adapt to a little syntactic sugar, but this proposal adds to the syntax of C# without providing a whole lot of value. There is value in consistency of the language, and having as few syntax rules as possible. I voted 👎

CyrusNajmabadi commented 1 year ago

@333fred Did we make on a decision on this feature? My recollection is that we may have explicitly decided to never do it. But my memory is very fuzzy here.

333fred commented 1 year ago

@CyrusNajmabadi https://github.com/dotnet/csharplang/blob/8889a8eb4ccb04b6ca74277dc567536d321b6137/meetings/2019/LDM-2019-09-11.md?plain=1#L252-L262 is our most recent discussion on it.

Thaina commented 1 year ago

I would like to disagree that, after using is not it was useful, but it was not overlapped with negate if

is not is great for negate one condition pattern in declarative manner. But this issue is about negating of the whole if condition parentheses. The use case is very difference

CyrusNajmabadi commented 1 year ago

On a personal level i went from wanting this a bunch of times to no longer wanting this. The reason, primarily, was that ! didn't compose with is. So you had to write if (!(x is Foo)) which was very ungainly. The ability for this to be absorbed into a pattern negated (pun intended) the need for the special statement form.

But this issue is about negating of the whole if condition parentheses. The use case is very difference

Except that for other conditions the ! can be absorbed into the expression . e.g. if (!(a || b)) is easy to write idiomatically as if (!a && !b). So the presence of if (!( has basically gone to almost nothing for me.

Thaina commented 1 year ago

if (!a && !b) is as ugly as if (!(a || b))

The key is the condition are not always short and simple as a and b. There was a bunch of property and function and there can be more than 4 in one if block. The ability to put the negation in front are very obvious. While checking that we need to negate multiple condition is error prone. It not only just ! it can be > == is and etc

For instance

if(!myObject0.Value.Condition || !myObject1.Value.CheckCondition() || myObject2.Component is not SomeType || myObject3.GetSomething() == null)
{
}

/// compare with

if !(myObject0.Value.Condition && myObject1.Value.CheckCondition() && myObject2.Component is SomeType && myObject3.GetSomething() != null)

This is the real situation

Another common pattern I do is

if(!(obj is SomeType someObj && someObj != null && someObj?.Value > 0))
    someObj = GetNewObj();
CyrusNajmabadi commented 1 year ago

// compare with

They seem equivalently readable to me. the ! + || version seems clearer as it's how i would say it verbally. "it's not this or not this or not this etc."

ViIvanov commented 1 year ago

Another common pattern I do is

if(!(obj is SomeType someObj && someObj != null && someObj?.Value > 0))
    someObj = GetNewObj();

It looks like the equivalent code looks much better for me (obj is SomeType will never true when obj is null):

if(obj is not SomeType someObj || someObj.Value <= 0)
    someObj = GetNewObj();
CyrusNajmabadi commented 1 year ago

Or just: if (obj is not SomeObj { Value: > 0 }).

TahirAhmadov commented 1 year ago

I think this can improve readability by a tiny bit, but it's not worth it. I think VS supports inserting a space after ( and before ) in an if statement, further reducing the need to make a language change here.

Thaina commented 1 year ago

@ViIvanov Sadly not in unity and some framework that will typecast itself to null if it was disposed

CyrusNajmabadi commented 1 year ago

That's seems to be a unity issue.

Thaina commented 1 year ago

If this could count as issue then I would say allow equal operator to compare with null is the bug in language design itself

HaloFour commented 1 year ago

@Thaina

If this could count as issue then I would say allow equal operator to compare with null is the bug in language design itself

Comparing a reference to null is idiomatic C# as it was idiomatic C++. Unity is the only framework that explicitly breaks the model here.

Thaina commented 1 year ago

@Thaina

If this could count as issue then I would say allow equal operator to compare with null is the bug in language design itself

Comparing a reference to null is idiomatic C# as it was idiomatic C++. Unity is the only framework that explicitly breaks the model here.

I mean overload operator

HaloFour commented 1 year ago

@Thaina

I mean overload operator

Why would that be a bug with the language? It's up to the implementation of the operator to apply appropriate semantics, and that would have to handle null whether it was a literal or an expression. IMO it'd be very wrong for the language to have two different behaviors depending on whether you used a null literal vs. an expression.

Thaina commented 1 year ago

@Thaina

I mean overload operator

Why would that be a bug with the language? It's up to the implementation of the operator to apply appropriate semantics, and that would have to handle null whether it was a literal or an expression. IMO it'd be very wrong for the language to have two different behaviors depending on whether you used a null literal vs. an expression.

As for me I don't really count it as a bug. But to allow it then call the one who do it as issue or breaks the model is not agreeable

I can also say unity is not the only framework that want to do this. To make something become null by itself after dispose is actually sensible and convenient. But the best they can do within C# rule is to just fake it with overload operator like this. And it work fine for more than 10 years

Anyway, this is actually nitpicking distraction from the start. I just tell someone that there is this possibility, it not even a main point I have presented, even without that == null there can be even more complicate condition. The one who attack unity as if it was their fault is you @CyrusNajmabadi and @HaloFour

HaloFour commented 1 year ago

@Thaina

I can also say unity is not the only framework that want to do this. To make something become null by itself after dispose is actually sensible and convenient. But the best they can do within C# rule is to just fake it with overload operator like this. And it work fine for more than 10 years

It's not fine, which is why folks keep asking Unity and C# to fix it somehow. It's why Unity objects don't work with reference equality throughout the runtime, not just with C# features. Breaking the spec is not a good thing, and this is very much Unity's fault.

CyrusNajmabadi commented 1 year ago

If you can't compare with null, how would strings implement their == operator?

Thaina commented 1 year ago

folks keep asking Unity and C# to fix it somehow. It's why Unity objects don't work with reference equality throughout the runtime

There are some that was obvious bug from unity side but as far as I can see many want C# to fix itself to allow unity's approach (such as https://github.com/dotnet/csharplang/discussions/2020). It really not as broken as JavaScript

Breaking the spec

This would be my bad but I never see a spec state that we must not compare with null inside overload operator for equal. People even use overload operator to compare difference class with the same record without care about reference equal

HaloFour commented 1 year ago

@Thaina

There are some that was obvious bug from unity side but as far as I can see many want C# to fix itself to allow unity's approach (such as https://github.com/dotnet/csharplang/discussions/2020)

Yes, like I said, some folks want C# to change how it works to match Unity's bizarre behavior here. The team doesn't seem terribly interested given that the feature basically has a single use case, specifically to work around Unity's bizarre behavior.

This would be my bad but I never see a spec state that we must not compare with null inside overload operator for equal.

Using it to replace reference equality would be abusive at the very least, which is why it has created such a huge amount of friction for Unity developers since it breaks everything that expects reference equality.

Thaina commented 1 year ago

feature basically has a single use case

using also has single use case, to defer the dispose of variable. Yet it was very useful. I can say the same for ability to treat null and disposed as the same state. It lessen the amount of code we need to write as null check everywhere

would be abusive at the very least it has created such a huge amount of friction

I can agree with this that unity had abuse C# language in many places but as I said this was allowed from the language and it was worth making framework become so much easy to write a check for state of object. This was one feature that introduce C# to many people to become developer. When object was destroyed it would considerably the same state as null is very sensible for common folk

To be fair there was many limitation in C# itself too to make unity require many abusing approach (such as coroutine with yield since the time before async/await) but to call it breaks the model is like calling System.Collections break the model of C# since we actually should use generic

To be honest I think this feature of unity is very far ahead of C#. The best C# has offer for this situation is string.IsNullOrEmpty which is not even an extension method (which means it need to reference the string class). While unity ability to check availability of its every object with one same pattern is powerfully simple

Same go with this feature request. When we want to create block of negation of the same logic. Ability to switch between line of if to line of negate if make us don't need to rewrite the whole line by simply add the negation

Right, this is another use case. Suppose we have write an if else block but then we want to swap the block. If we have ability to change just one character in front of the block in addition to dragging the whole block up and down will be very convenient