Question: How is !! envisaged to be used? #5735
-
After David Fowler posted this GitHub issue on Twitter I have discovered the I wanted to ask how the C# language team envisaged this new operator to be used? If I understand this correctly this is a short hand form to essentially replace Essentially, unlike the nullable (e.g. My first question is, are my above assumptions correct? If yes, is this simply a new operator which converts random unhandled The second question I have is how is this new operator supposed to be used alongside the nullable feature? Here's what I find strange. If I was to write "clean" code then I would simply only have two options for declaring a method argument:
There is no middle ground (a little bit null, but not too much null) if you see what I mean. So in connection with the nullable feature I essentially end up decorating each and every argument across an entire codebase with either On the other hand I could be wrong, and the new So you can see I struggle to really get my head around how this new operator is supposed to be used and how it is meant to fit into the rest of C#. Would you be able to shed some light on this and feel free to correct me where I'm wrong :) Thank you! 🙏 |
Beta Was this translation helpful? Give feedback.
Replies: 60 comments 660 replies
-
The proposal and discussion are here: #2145 TL;DR: NRTs don't change the behavior of code, they only provide information to the compiler to allow it to warn on unexpected attempts to pass Null validation is intended to replace the code currently required to do null argument validation in methods, and is it intended to have an impact on the behavior of the method. It does replace what might be a |
Beta Was this translation helpful? Give feedback.
-
I hope they don't turn C# to be like Kotlin. Kotlin enforces this weird nullsafety """best practives""" that make code worse. But what about nullable value types? |
Beta Was this translation helpful? Give feedback.
-
I think that adding annotations to the type, in the form of punctuation symbols, has low usability. We don't make variable names have special meaning anywhere else, and the industry knowledge about the failure modes that Hungarian notation set up for doing this by convention suggests this does not work out well. If the goal is to provide a runtime check on a property then using a [NotNull] attribute on the parameter would be far more in keeping with the established idioms of C#. Use of !! feels like it is bolted on from another language, and goes further to worsen the belief that C# is becoming a "kitchen sink language" where everything is just tossed in, regardless of whether it suits the idioms of the language. The usage of !! on a variable name fails the "principle of least surprise" as it introduces idioms that don't fit with the rest of the language. Developers won't have trained their eye to look for symbols in parameter names, so this is likely to go unnoticed, whereas the use of attributes on parameters is understood. Finally using [NotNull] fits with earlier exploration of Code Contracts and would open up the possibility of exploring what value that space could offer, although I recognize earlier attempts on Code Contracts were not widely adopted |
Beta Was this translation helpful? Give feedback.
-
Please just add |
Beta Was this translation helpful? Give feedback.
-
There are two things to that..
Now what we lack is "I am reference type but doesn't allow null"... and So why on Earth we cannot have |
Beta Was this translation helpful? Give feedback.
-
But from a usability perspective it appears to be part of the parameter name, folks aren't used to seeing a postfix on a parameter. That may not be what it is, but it is how it appears. An explicit notnull keyword would be better if we are concerned about the usage of an attribute at runtime - although the use of attributes at runtime is common in frameworks. Using a symbol that appears to be part of the name looks like syntactical noise.
That is often a warning sign that the design is problematic... |
Beta Was this translation helpful? Give feedback.
-
It may be simpler, but I am not sure it is simple. It's possible that this change has a higher cost to value for the BCL where there maybe many more checks on null arguments than there are in many app developer codebases, and as such optimizing for a short postfix operator may seem better than optimizing for clarity and sympathy with expectations of syntax. But this is sufficiently 'hidden' from the way most C# developers write code, that I would issue style guidance to any team I worked with to not use it. However, a notnull keyword would be obvious, expected, and add real value.
When you make something syntatic sugar your goal, such as with async, is to remove complexity from developers. I would suggest that the conditional block does not represent complexity. It may represent a burden to regularly have to write it. If there is a burden to writing it - it's just boilerplate code - then a more typical answers would be to use attributes for code generation or generic types. The addition of syntactical elements just creates further complexity around the language - developers now have to be alert to postfix operators on arguments. So it feels like we have added complexity, when what we needed to create was not simplicity, but simply remove the burden of writing code. There are more obvious solutions to that. It's this test - the obviousness of the solution - that I think this fails. |
Beta Was this translation helpful? Give feedback.
-
May have suggested this to be a flag that we can set on the csproj as
or was this or something similar considered as an option, if so just curious why was that approach not selected. As I see it that would have improved the existing nullable reference type experience and would have been easier to adopt as well on application code if the nullable annotations are already there. |
Beta Was this translation helpful? Give feedback.
-
|
Beta Was this translation helpful? Give feedback.
-
I'm going to copy here what I wrote on the implementation MR thread: Please stop adding weird symbols for nullability reasons. Everything is already confusing enough with using the same nullability symbols for value and reference types ( The feature itself is fine, but it could be easily added with something like: void M([ThrowIfNull] string a) {} Which is explicit, and readable... because the next thing you know, we'll be reading the language when loud as questioning and shouting stuff ( This is all making the language completely unreadable And yes, I understand You could even use void M(notnull string a) {} Which doesn't add any new keywords ( |
Beta Was this translation helpful? Give feedback.
-
I'm still strongly against this feature.
void Foo(string bar)
{
if (string.IsNullOrEmpty(bar)) throw new ArgumentException(nameof(bar)); //Extremely common to want to guard against empty or whitespace strings
}
void Foo2(string bar)
{
bar ??= "howdy do"; //Extremely common to want to default a value if its null since parameter defaults are required to be compile-time constants. null allows us to work around that limitation
}
|
Beta Was this translation helpful? Give feedback.
-
A bit tangential but I do really hope that those individuals who are taking an interest in the design process of the language stick around. The feedback is welcome and this process does happen out in the open. The linked issue and meeting notes hopefully give some insight into how the sausage is made. It may seem like that feedback is hitting a wall at the moment but I think that's largely due to this particular feature having already been shepherded pretty deep through the design process and already merged into the Roslyn codebase. I am not on the language design team nor do I speak for them. |
Beta Was this translation helpful? Give feedback.
-
I think "method contracts" feature aims to solve this problem. |
Beta Was this translation helpful? Give feedback.
-
My biggest issue with this is that it makes it extremely difficult to read the code and it isn't intuitive. From a readability perspective, the examples already mentioned numerous times above show how convoluted the syntax becomes. Additionally, when doing code reviews, reasoning about the parameters in this way is very difficult. Should the code be formatted across multiple lines or use long parameter names, these clumsy operators get missed. For example: public static Task<Patient> GenerateNewInPatientRecordAsync(Person patientPersonRecord!!, Person? accountHolderRecord, bool isPatientSameAsAccountHolder!!, CancellationToken cancellationToken = default) I just feel the above kind of example happens so frequently in general code bases that reviewing the code and reasoning about it down the line is incredibly difficult. Is there not a more intuitive operator we could use? Maybe Secondly, I've spent so much time clarifying over and over again to our more junior devs that |
Beta Was this translation helpful? Give feedback.
-
We get that, but are providing feedback on usability. I understand the 'please provide feedback on language features' request, but this feature was not trailed in a way that might attract feedback as many others were. It can be tough for anyone to find the time to monitor all the changes, unless there is some kind of heads up that we see on it. This one really seemed to fly under the radar, and hence the surprise everyone is treating it with. I note you say that it is too late to provide this feedback, but that is the "sunk cost fallacy". Once you have added this complexity to the language, its a far greater magnitude of effort to undo it. Given that much of the feedback has not been "yeah, this is great" but "no, please don't" I'm not sure what goals that the team has this is furthering to proceed without a review of the feedback you are receiving. |
Beta Was this translation helpful? Give feedback.
-
A suggestion I made a long time ago (because I see It would have looked like this: void Deposit(string message, decimal, Account account)
where message is not null,
amount is >= 0,
Account is { Status: AccountStatus.Open }
{
...
} I think my suggestion was mistakenly assumed to be advocating for contracts and therefore discarded. Yes, this syntax may have been forward compatible with code contracts, but my suggestion was purely about expression of guard clauses. I reused the where keyword because we already use it in similar ways, though maybe the run-time vs compile-time difference would have needed a different keyword. |
Beta Was this translation helpful? Give feedback.
-
In my opinion this syntax is hard to understand. |
Beta Was this translation helpful? Give feedback.
-
Was
or
Is it too late to suggest to strongly reconsider introducing such an opaque sigil? |
Beta Was this translation helpful? Give feedback.
-
(Mandatory disclaimer: I'm fine with I'm wondering if some of the readability benefits of the "contract" syntax various people have proposed here could be achieved without needing to come up with metadata representations etc if there were a convenient shorthand syntax that used patterns and applied some heuristics to follow standard conventions. For example: public void Foo(string str)
{
// equivalent to 'if (str is null) throw new ArgumentNullException(nameof(str));'
// applies if the LHS is an argument to the method that has definitely not been modified and the RHS is literal null
throw if str is null;
} It should be possible to have similar heuristics for |
Beta Was this translation helpful? Give feedback.
-
After reading @jaredpar 's summary in another reply, I think I pretty much agree that suffix
There's a lot of emotions and gut feelings in here, and it's simple to dismiss them as being free of valid information and say that it's impossible to base decisions on, or plan schedules around, them. But I think listening to people's concerns and taking the collective, reasonably unison reaction can be instructive. (There's certainly been enough C# changes in the past few years that the people reacting now would have reacted a long time ago if this was just about "someone moving the cheese" or "not liking change".) In recent years Microsoft has felt out of touch with the community in a few instances (like the removing-Hot-Reload debacle). Microsoft is driving the C# language design in a process that's not a democracy, and that is usually a process with a successful and productive outcome. Things feel holistic, well put together, well thought through, with a through-line going from the past into the future. But: I have been in many situations with other projects, as I'm sure many developers have, where there have been little features that haven't paid their weight or fit in or have actively been considered to be bad ideas by developers, but been implemented anyway because of clout or politics. Basically, this feature, although I am very much not making any accusations of the actual process, gives off a similar whiff and a similar sinking feeling, by many small indications in combination. |
Beta Was this translation helpful? Give feedback.
-
But what if that's not the truth? What if what we've learned makes us feel we don't want to go that direction. I don't want us to make placating statements that can give false hope. When we make statements about the future, it's important to me that they are realistic. Otherwise, all we'll have to do is just say the above any everything. And I don't think that will satisfy people. Indeed I think it will be worse. E.g. "you always say that you're interested in the bigger thing, but 98% of then don't happen, so stop with the false hope". C# is, above all, pragmatic. Even with a community, the velocity we can achieve is both limited (and simultaneously, too fast for some). We aim to hit a reasonable sweet spot with what is possible to have large steps forward while also continually focusing on things like narrow syntactic niceties. I see you dismissed a prior example of this. Because, of course, that's what people do if they happen to like a particular syntactic improvement. But every time we do that, we hear the same feedback from people who do not benefit from it or who do not want to use it. It's always: stop with the small stuff that I don't need, work in the big things I do need. But, similarly, every large feature comes with the contrasting feedback of: this is such a big change for the language, can you please just focus on helping me reduce boilerplate with syntactic helpers. This is why we try to blend both. We have millions of users with varying needs and desires. So we try for a set of features that can hit that within the available budget and velocity that is possible. |
Beta Was this translation helpful? Give feedback.
-
This functionality does not solve any problems, but it complicates the reading of the code. Most often, we have to read the code, not write it. It can be implemented using attributes, such as [ThrowIfNull], which is more semantically and explicitly, and is suitable for implementing other checks (for example, that Enum is defined or the collection is not empty). This is a step towards turning C# into an unreadable analogue of Kotlin. |
Beta Was this translation helpful? Give feedback.
-
It's more compelling to me to write more code than messing with readability. |
Beta Was this translation helpful? Give feedback.
-
I'm not a fan of void Do(int? x, string? s!!) => Get(s)!.Run();
But I understand the pain. This null check boilerplate is a pain and shows up everywhere. public static bool Between<T>(this T foo, T a, T b)
where T: IComparable<T>
{
if (foo is null)
throw new ArgumentNullException(nameof(foo));
if (a is null)
throw new ArgumentNullException(nameof(a));
if (b is null)
throw new ArgumentNullException(nameof(b));
return foo.CompareTo(a) > 0 && foo.CompareTo(b) < 0;
} However I agree with others above opining that the void Do(int? x, string? s) throw: s! => Get(s)!.Run(); public static bool Between<T>(this T foo, T a, T b)
where T: IComparable<T>,
throw: foo!, a!, b! =>
foo.CompareTo(a) > 0 && foo.CompareTo(b) < 0; I'm also a fan of @bjorg 's proposal if anyone interested in this topic hasn't glanced at that. It feels like a fuller solution whereas But with all that said, we have attributes in C# that change behavior all the time. int Get(string? s)
{
if (s == null) throw new ArgumentNullException(nameof(s));
if (!User.Authorized()) { throw ... }
} Code review says we got attributes for that auth check! Pull that [Authorized]
int Get(string? s)
{
if (s == null) throw new ArgumentNullException(nameof(s));
...
} We even have magic attributes like [Authorized]
int Get([ThrowNull] string? s)
{
...
} |
Beta Was this translation helpful? Give feedback.
-
A lot of comments here, hard to read all of them. I was pretty disturbing with the null operator I am not completly against this operator specificly, there are good reasons about it, but contributors have to be really carrefull if they want to implement more of them in the future. To finish, I agree with a previous post. Using |
Beta Was this translation helpful? Give feedback.
-
If you add !! in expressions, can we agree that there should also be a standard compiler warning on the usage of !, that can be enabled for green field projects? (Can be suppressed around certain code blocks with pragmas) Think about it:
Not a good sign when the programmer's path of least resistance is unsafe --> safe |
Beta Was this translation helpful? Give feedback.
-
Thanks for explaining it.
Outside of what it can understand today, I assume. Well, I haven't hit this case of "doesn't match target type" yet, but I personally would do this:
Which is greppable and would cause a code review... just like we do with our |
Beta Was this translation helpful? Give feedback.
-
Would this be as equivalent safety wise?
|
Beta Was this translation helpful? Give feedback.
-
Quite a few people on reddit think .Value and .HasValue work on NRTs like |
Beta Was this translation helpful? Give feedback.
-
Why not executable annotations? [ThrowIfNull] is more elequent and indicative than '!!'. void Do([ThrowIfNull] string action)
{
}
[return: ThrowIfNull] string SomeString()
{
return null;
} [ParameterInterceptor] could be added to Attribute subclass to indicate that compiler should inject .Intercept(ref stack-pointer, string name) on arguments when methods are called and returned. JIT can inline this.
[ParameterInterceptor, AttributeUsage(AttributeTargets.Parameter | AttributeTargets.ReturnValue)]
public class ThrowIfNullAttribute : Attribute
{
[DebuggerHidden, MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Intercept(ref object value, string name)
{
if (value is null) throw new ArgumentNullException(name);
}
}
[ParameterInterceptor, AttributeUsage(AttributeTargets.Parameter | AttributeTargets.ReturnValue)]
public class ThrowIfDefaultAttribute<T> : Attribute
{
[DebuggerHidden, MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Intercept(ref T value, string name)
{
if (value is default) throw new ArgumentNullException(name);
}
} [ThrowIfDefault] would be for value-types. void Do([ThrowIfDefault] int id)
{
}
[return: ThrowIfDefault] int SomeInteger()
{
return 0;
} Besides validation, interceptors have broader customization potential as they can modify the stack slot. void AddKey([AssertPrefix] string key)
void Add([DefaultOnNull("")] string value);
[return: DefaultOnNull("")] string Do();
void Do([TraceHere] Exception e);
void Do([Abs] int value);
void Do([Decorate] IMyInterface value); |
Beta Was this translation helpful? Give feedback.
Just got off a .NET community standup where the team addressed many issues here and specifically my questions asked.
I'll leave the link to that recording here for people to consume and mark it as the answer:
https://www.youtube.com/watch?v=Fz4hViH5bGc