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

Null-conditional operator doesn't work with Nullables #572

Closed
VBAndCs opened this issue Sep 24, 2020 · 44 comments
Closed

Null-conditional operator doesn't work with Nullables #572

VBAndCs opened this issue Sep 24, 2020 · 44 comments

Comments

@VBAndCs
Copy link

VBAndCs commented Sep 24, 2020

Dim d As Date? = Nothing
Console.WriteLine(d?.Value)

I get the error:

Value is not a member of Date

Why doesn't the ? operator work with nullable values?
I think it should.

@VBAndCs
Copy link
Author

VBAndCs commented Sep 24, 2020

C# has the same issue too!

@YairHalberstadt
Copy link

The ? Operator turns it from a Date? to a Date so you don't need to use Value you can just access the members directly.

@VBAndCs
Copy link
Author

VBAndCs commented Sep 24, 2020

This is confusing!
What If I want to do this:
If d?.Value > Now Then .....
This forces me to:
If d.HasValue AndAlso d.Value > Now Then .....
So, it is not only confusing design, but inefficient, and took away the basic benefit of the ? operator!

@YairHalberstadt
Copy link

YairHalberstadt commented Sep 24, 2020

The ? Operator doesn't work for classes either for operators.

@VBAndCs
Copy link
Author

VBAndCs commented Sep 24, 2020

I suggest to allow accessing the Value property beside the Date members in this special case.

@VBAndCs
Copy link
Author

VBAndCs commented Sep 24, 2020

The ? Operator doesn't work for classes either for operators.

VB has its logic. This will run:

        If Nothing > Now Then

        End If

as Nothing will be considered the default DateTime

@VBAndCs
Copy link
Author

VBAndCs commented Sep 25, 2020

Got it but still confused :)

@VBAndCs VBAndCs closed this as completed Sep 25, 2020
@hartmair
Copy link

If d.HasValue AndAlso d.Value > Now Then ...

You can just write

If d > Now Then ...

@VBAndCs
Copy link
Author

VBAndCs commented Oct 6, 2020

I faced a practical situation in #576 related to this.

result.ToList.Append(New With {
       Key .ValidDate = If (rcvValidDate is Nothing, Nothing, rcvValidDate.Value), 
       Key .Amount = CDec(rcvAmount)
    }
)

the Append method expects an anonymous type (key Date, key Decimal) and I want to pass a nullable date to it:

  • I can't use .ValidDate = rcvValidDate because the resultant nullable will be (key Date?, key Decimal)
  • I can't use .ValidDate = rcvValidDate.Value because it can throw an exception if rcvValidDate is nothing.
  • I can't use .ValidDate = rcvValidDate?.Value because it is not allowed!!

So, I have to use this weird long confusing expression:
.ValidDate = If (rcvValidDate is Nothing, Nothing, rcvValidDate.Value)

Of course anonymous types should be enhanced, but nullable values should also allow ?.Value just in unexpected cases like this.

@VBAndCs VBAndCs reopened this Oct 6, 2020
@YairHalberstadt
Copy link

result.ToList.Append(New With {
       Key .ValidDate = rcvValidDate, 
       Key .Amount = CDec(rcvAmount)
    }
)

Does this not work?

@YairHalberstadt
Copy link

Perhaps you want rcvValidDate.GetValueOrDefault()?

@VBAndCs
Copy link
Author

VBAndCs commented Oct 6, 2020

No. It gives:

Error BC30311 Value of type '<anonymous type: Key ValidDate As Date?, Key Amount As Decimal>' cannot be converted to '<anonymous type: Key ValidDate As Date, Key Amount As Decimal>'

but this works:

result.ToList.Append(New With {
       Key .ValidDate = CDate(rcvValidDate), 
       Key .Amount = CDec(rcvAmount)
    }
)

@VBAndCs
Copy link
Author

VBAndCs commented Oct 6, 2020

Perhaps you want rcvValidDate.GetValueOrDefault()?

CDate(rcvValidDate) is better. But rcvValidDate?.Value would be consistent with the general way we are used to. The compiler can just convert it to CDate(rcvValidDate) (because of course rcvValidDate will cause an exception when dealing with anonymous types)!

@MrGadget1024
Copy link

Sorry to hijack but I ran into something similar this week:

Public Class Foo
	Public Property BarList As List(Of String)

	Public Function ShouldSerializeBarList() As Boolean
		' Fails
		Return BarList?.Count > 0

		' Works
		Return BarList IsNot Nothing AndAlso BarList.Count > 0
	End Function
End Class

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

I faced this situation before, and we had long discussion about it. but I can't find the link to the issue. VB.NET returns a nullable Boolean (Booleabn?) from any comparison contains Nothing as an operand. So, the BarList?.Count will be Nothing, and Nothing > 0 well return a Boolean?, and when vb tries to convert it to Boolean, it seems it uses the Boolan?.Value that causes the bug. In my opinion, this shouldn't happen, as Boolean can accept Nothing as the default value (False), so, VB should check this case and get do the conversion safly. Maybe you open a new proposal for allowing assigning Nullable values to their Value types safely.

@CyrusNajmabadi
Copy link
Member

		' Fails
		Return BarList?.Count > 0

		' Works
		Return BarList IsNot Nothing AndAlso BarList.Count > 0

these are not hte same. the first is (effectively in pseudocode):

if BarList is null, return null.
otherwise, return if barlist.count is greater than 0.  

This means you can return either null or a boolean (or, a Nullable(of boolean)). As you method says it returns Boolean, this is a problem as you are returning a wider type from a narrower type (similar to trying to return an object from a method you say only returns strings).

The second says:

if barlist is nothing, return false. 
otherwise return if barlist.count is greater than 0.

This means you only ever return a boolean. This is ok as that matches your method.

There are many ways you can fixup your code. Which is "the right way" depends entirely on what you're trying to do.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@CyrusNajmabadi
A Nullable Boolean has to possible values:
1- A Boolean value
2- Nothing, and it can be assigned to Boolean , and Nothing in VB is considered the default value of value types.

so, in either case, the conversion from Nullable Value to the normal value is valid, and it is wrong to treat VB as C# in this situation.

@CyrusNajmabadi
Copy link
Member

@VBAndCs I'm sorry, this is incorrect, and is the same line of thinking as in dotnet/roslyn#48387. In VB, it is a narrowing conversion to go from T? to T. It says so right here:

https://github.com/dotnet/vblang/blob/master/spec/conversions.md#narrowing-conversions

Nullable Value Type conversions
From a type T? to a type T.

As such, with option strict on, you will get an error if you try to do this. Once again, the language spec states how this works.

and it is wrong to treat VB as C# in this situation.

I never said anything about C#. I was speaking entirely about how VB treats things here. I highly recommend you read through the other issue and also through the language specification here. It seems like it would be very helpful for understanding this issue and not getting tripped up over this concept. Thanks!

@gilfusion
Copy link

Just wanted to mention a couple of notes on things from across the discussion.

First, remember that ?. is a "null-propagating" operator. It always results in a nullable value. Reference types stay the same, but value types are wrapped in Nullable(Of T) whenever ?. is used. (Someone please correct me if I missed an exception to this.) Therefore, using ?.Value to get the underlying type makes no sense to me. (If foo.Value is used to get a Date from a Date?, but foo?.anything turns a Date into a Date?, wouldn't a hypothetical foo?.Value just get the original foo as its original Date? type?)

Next, when doing math or comparison operations with nullable values (BarList?.Count results in an Integer? or Nullable(Of Integer)), it's helpful to look at the Nothing case as "I don't know". Is 10 > 0? Yes. Is 0 > 0? No. Is "I don't know" > 0? Well, I don't know. Therefore, the result of BarList?.Count > 0 is also a nullable value, Boolean? or Nullable(Of Boolean).

Now, when you want to turn that Boolean? back into a regular Boolean, there are several options, with varying results:

Expression If Not Nothing If Nothing
foo.Value True or False Exception thrown
CBool(foo) True or False Exception thrown
foo.GetValueOrDefault() True or False False (default for Boolean)
foo.GetValueOrDefault(bar) True or False bar
If(foo, bar) True or False bar

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@CyrusNajmabadi

As such, with option strict on, you will get an error if you try to do this. Once again, the language spec states how this works.

Fantastic! The exception happens even when option strict off which is the default in VB by the way.
So, this is defiantly a bug!

But this goes beyond option strict. This code will throw despite using the CBool:

        Dim b1 As Boolean? = Nothing
        Dim b2 As Boolean = CBool(b1)

What I am saying here. is that the unlike C#, conversion from vtype? to vtype is possible in vb and perfectly safe, and should be allowed implicitly and explicitly.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@gilfusion
All this makes sense in C#, but not in VB. How can we accept this to throw:

        Dim b1 As Boolean? = Nothing
        'Dim b2 As Boolean = CBool(b1) ' Exception

but this runs OK:

        Dim b1 As Boolean? = Nothing
        Dim b3 = If(b1, Nothing)

despite that both CBool(b1) and If(b1, Nothing) return Nothing?!
This is insane, and VB should allow b2= CBool(b1) and even b2 = b1 when option strict off.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@gilfusion @CyrusNajmabadi
OK. let's put it this way:
VB already converts Boolean? to "Boolean" successfully despite Option strict on or off. How is that?. Try:

        Dim b1 As Boolean? = Nothing
        Dim b2 As Boolean
        If b1 Then
            b2 = true
        Else
            b2 = False
        End If

in the expression If b1 Then, b1 is converted successfully to Boolean to evaluate the condition. And as I said, this works even with Option strict On.
So, VB must fix the CBool, (which I think it calls behind the scene when option strict off
This issue causes a lot of confusion to VB programmers, and wastes a lot of their time, because it breaks there expectations, and the language common sense.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

This also woks fine even with `Option Strict On'

        Dim b1 As Boolean? = Nothing
        Dim b2 As Boolean
        If Not b1 Then
            b2 = False
        Else
            b2 = True
        End If

but b2 = Not b1 gives a design time error, (and a runtime error if option strict off)!
How come to accept If Not b1 Then but refuse b2 = Not b1 ?!!

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

In fact, all operation on nullable Boolean works fine in f statement:

        Dim b1 As Boolean? = Nothing
        Dim b2 As Boolean
        If b1 AndAlso b2 Then
            b2 = True
        ElseIf b1 OrElse b2 Then
            b2 = False
        End If

but fails elsewhere.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

Note that C# doesn't accept !b1, and this is expected because the logic differs between the two languages

bool? b1 = null;
bool b2;
if (!b1)
  b2 = false;

@gilfusion
Copy link

@VBAndCs, ultimately, I think the confusion goes back to the VB7/8 timeframe went from one meaning to three.

Here is some VB6 code I was just tinkering with:

Dim myVar As Variant
Dim myBool As Boolean
    
Private Sub cmdEmpty_Click()
    myVar = Empty
    myBool = myVar
End Sub

Private Sub cmdNothing_Click()
    myVar = Nothing
    myBool = myVar
End Sub

Private Sub cmdNull_Click()
    myVar = Null
    myBool = myVar
End Sub

Back then, Nothing was only used for object references, so the code using it is an error, because either I'm not using the object assignment statement, or because a Boolean is not an object. (Different times back then.)

In VB7 (.NET 1.0), the meaning of Nothing was combined with Empty. Empty was the expression/literal that meant the default value for the type. The code above using Empty works the way you would expect, leading to the myBool variable being False. VB6-style Null was not available in VB7.

In VB8 (.NET 2.0), nullable types were added, and, when using nullable intrinsic types, Nothing behaved like the classic Null. Null back then and Nothing today (in the context of nullable types). If you tried to store Null in a non-nullable variable, it resulted in an error. If you used an operator with Null, most of the time, the result would be Null (like I said, Null was treated as "I don't know", but if there were cases were you could know the result regardless of the null value, VB could/can give you that result). As for If, VB didn't run the If block if it could be False, so, in the null case, neither the body of If myVar... nor If Not myVar... would have run.

So, the problem I think you're having with Nothing is that you want it to behave like old VB Empty at a time when it is behaving like old VB Null. Neither of these is a "C# thing"--both have been a part of VB since long before a single line of C# was written. Having multiple "non-values" was ridiculed back in the day, but maybe unifying them was a mistake. Is there a way to re-separate them? Probably not without breaking a lot of code that has been in the last couple decades.

@MrGadget1024
Copy link

MrGadget1024 commented Oct 7, 2020

Just wanted to mention a couple of notes on things from across the discussion.

First, remember that ?. is a "null-propagating" operator. It always results in a nullable value. Reference types stay the same, but value types are wrapped in Nullable(Of T) whenever ?. is used. (Someone please correct me if I missed an exception to this.) Therefore, using ?.Value to get the underlying type makes no sense to me. (If foo.Value is used to get a Date from a Date?, but foo?.anything turns a Date into a Date?, wouldn't a hypothetical foo?.Value just get the original foo as its original Date? type?)

Next, when doing math or comparison operations with nullable values (BarList?.Count results in an Integer? or Nullable(Of Integer)), it's helpful to look at the Nothing case as "I don't know". Is 10 > 0? Yes. Is 0 > 0? No. Is "I don't know" > 0? Well, I don't know. Therefore, the result of BarList?.Count > 0 is also a nullable value, Boolean? or Nullable(Of Boolean).

Now, when you want to turn that Boolean? back into a regular Boolean, there are several options, with varying results:
Expression If Not Nothing If Nothing
foo.Value True or False Exception thrown
CBool(foo) True or False Exception thrown
foo.GetValueOrDefault() True or False False (default for Boolean)
foo.GetValueOrDefault(bar) True or False bar
If(foo, bar) True or False bar

So based on the above, this works with the XmlSerializer in all cases: null list, empty list, non-empty list

Public Class Foo
    Public Property BarList As List(Of String)

    Public Function ShouldSerializeBarList() As Boolean
        Return (BarList?.Count > 0).GetValueOrDefault()
    End Function
End Class

So what's the verdict...bug or not?

Edit:
Would there be any performance difference between the above and the long-hand version:
Return BarList IsNot Nothing AndAlso BarList.Count > 0

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

@gilfusion
I agree with most of what you said, but there is a real contradiction in dealing with nullable Boolean, that it breaks even what you yourself expect! Try this:

        Dim b1 As Boolean? = Nothing
        If Not b1 Or True Then
            Console.WriteLine("b1 is false")
        Else
            Console.WriteLine("b1 is true")
        End If

You expect to see: b1 is true as you think that any comparison involving Nothing will yield nothing, but actually, Not Boolean? yields Nothing that is considered False in the or operation!!!, So, the result will be: B1 is false!!!
So, it is Nothing that means false after all, but if used alone, If Not B1 Then will be Nothing that means null!
This is a total mess!

@VBAndCs
Copy link
Author

VBAndCs commented Oct 7, 2020

My suggestion here, to give error on any condition using nullable directly without .value or .GetValueOrDefault as the results are not expected!
And my advise is change this behaviour in any new implementation of the language.

@pricerc
Copy link

pricerc commented Oct 7, 2020

change this behaviour in any new implementation of the language.

This is where it would have to happen.

VB has always been very strong on backward compatibility and not introducing breaking changes. VB today works with nullables the way it has for years. And we know that Microsoft are not about to change it.

This discussion, along with the discussions around 'non-nullable' vs 'nullable' reference types (mostly in C#, but also here #355 and #386 ), suggests to me that some of these concepts were not fully considered at the time .NET was being developed, and both VB and C# have inconsistencies in their treatment of null-related activities.

However, we are where were are, and fixing it will likely break a lot of existing code.

Personally, I've don't have a problem with the status quo, because it is clear what the code is doing if you don't try to minimize the code. If you have a 3-state variable (Nullable(Of Boolean)), then you need to test all three states. If you don't need three states, then don't use a nullable boolean.

Dim b1 As Boolean? = GetTheValue()

If b1.HasValue Then
   If b1.Value Then
     ' do the true thing
   Else
     ' do the false thing
   End If
Else
   ' do the null thing
End If

has always been the way I deal with a nullable. Of any kind, because that's kind of the point of them.

As for:

If d.HasValue AndAlso d.Value > Now Then .....
So, it is not only confusing design, but inefficient, and took away the basic benefit of the ? operator!

That is completely subjective, and I disagree. It makes perfect sense to me. In fact, I find it is If d?.Value > Now Then ..... that is confusing to me.

If d doesn't have a value, then why would you compare it to another value? Remember that a syntax for a value type needs to be consistent for all value types. Does your proposal make sense if you replace 'Date' and 'Now' with any other value type instead of 'DateTime', and some other value instead of 'Now'.

I would suggest that the answer is 'no', because the semantics of such a construct would be completely dependent on what the author of the value type decides is the default value. Which need not be a constant.

Going back to your If d.HasValue AndAlso d.Value > Now Then ....., you could always abbreviate it like:

If If(d, DateTime.MinValue) > Now Then ...

which also allows you to pick the default value you want, instead of relying on the type's default. Since DateTime.MinValue is also the default value for a DateTime, it should also work here:

 result.ToList.Append(New With {
        Key .ValidDate = If (rcvValidDate, DateTime.MinValue), 
        Key .Amount = CDec(rcvAmount)
     }
 )

@CyrusNajmabadi
Copy link
Member

My suggestion here, to give error on any condition using nullable directly without .value or .GetValueOrDefault as the results are not expected!

This has been how the language has worked for the last 15 years. Any new behavior would be even more unexpected than the behavior which has been the norm since literally VB 8.0.

@CyrusNajmabadi
Copy link
Member

suggests to me that some of these concepts were not fully considered at the time .NET was being developed, and both VB and C# have inconsistencies in their treatment of null-related activities.

They were definitely considered at the time, and many options were considered. The choices we made here reflect our thinking back then on what we felt was appropriate for this sort of type.

@CyrusNajmabadi
Copy link
Member

This is a total mess!

VB booleans are a tri-state system. That's why you use them. If you just want two states, use Boolean. The purpose of Boolean? is specifically to allow for a third state. However, in order to use them you have to know how the tri-state logic works for all these operators.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 8, 2020

@CyrusNajmabadi

in order to use them you have to know how the tri-state logic works for all these operators.

This is exactly the problem: you invented different rules for each language! This is illogical :)

@VBAndCs VBAndCs closed this as completed Oct 8, 2020
@CyrusNajmabadi
Copy link
Member

This is exactly the problem: you invented different rules for each language

They are different languages. If they had the exact same rules there would be no point using one over the other. This is why multiple languages exist. :) If you just want C# semantics... use C# :)

@VBAndCs
Copy link
Author

VBAndCs commented Oct 13, 2020

Just another practical case that shows the agony caused by current design:
In this code:
Dim i= _ItemIDs.IndexOf(Value)
I get exception if ItemIDs is Nothing, so I tried:
Dim i= _ItemIDs?.IndexOf(Value)
But it raised exception too as the Integer? is nothing and must have a value!
So, I have to use:
Dim i As Integer = (_ItemIDs?.IndexOf(Value)).GetValueOrDefault()
which is less readable than:
Dim i = If (_ItemIDs is Nothing, 0 ,_ItemIDs.IndexOf(Value))
So, the ? doesn't make things easier or quite shorter in such a case.
Again: Things will be VBish if nullbale structure holding Nothing is converted to the default value of its value type, as VB normally does all the time.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 13, 2020

And another one:
Return Me.SelectedID <> (GetOriginalValueFunc?.Invoke).GetValueOrDefault

The expressions containing ? turns out to be longer and complex!

A side note:
Why doesn't VB editor auto add () after Invode and GetValueOrDefault methods?
It adds them after many other functions. Is there a rule for this?

@pricerc
Copy link

pricerc commented Oct 14, 2020

Dim _ItemIDs As List(Of Integer)
Dim Value As Integer = 10

Dim i = If(_ItemIDs?.IndexOf(Value), 0)

I'm too lazy to write a whole function for GetOriginalValueFunc, and you don't say explicitly what you're expecting. But I suspect this will work (or at least produce the same result).

Return Me.SelectedID <> If(GetOriginalValueFunc()?.Invoke(), 0)

Using a null-coalescing If(..) also gives you an opportunity to specify your own default that is not the Type's default, which I think makes it more useful than GetValueOrDefault().

@CyrusNajmabadi
Copy link
Member

Again:

Posting here won't change this. If you want the language to change, open a issue where this can be discussed and evaluated. Do not reuse existing issues (which have been addressed) to bring up new things you want changed.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 14, 2020

Why doesn't VB editor auto add () after Invode and GetValueOrDefault methods?

That question is better suited to dotnet/roslyn. VS ide behavior is not really part of the vblang repo. Thanks!

Note: if you wanted to create a PR to address this on dotnet/roslyn, it would very likely be accepted.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 14, 2020

@
Return Me.SelectedID <> If(GetOriginalValueFunc()?.Invoke(), 0)
Combining If and ? means writing 2 conditions. I don't find that a good practice, especially in huge loops. So, I will stuck with If(, , ).
Thanks.

@VBAndCs
Copy link
Author

VBAndCs commented Oct 14, 2020

@CyrusNajmabadi
Sorry. We stopped hoping for fixing anything years ago, so, we just make conversation between the community members, aiming to perfect the design of the language in new implementations that already in motion, or expected to come soon.

I posted the issue in Roslyn: dotnet/roslyn#48581
Thanks.

@CyrusNajmabadi
Copy link
Member

We stopped hoping for fixing anything years ago

We accept dozens of community contributions per sprint. These also include many VB improvements. In general, we only take improvements if they also handle the VB side as well. And if that's out of scope for the community member, we'll pitch in to do that work ourselves to ensure that VB and C# both get the improvement.

@CyrusNajmabadi
Copy link
Member

so, we just make conversation between the community members

This is offtopic for this issue. If you want to talk about new things please either file a new issue, or take the discussion to a community forum like gitter or discord. Future posts on this issue that continue this will be marked off topic. This is important as it affects our ability to manage and track these topics through issues. By taking things off track, or using closed issues, it effectively subverts that ability. Thank you.

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

7 participants