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

Make (condition ? 1 : null) just work #11886

Closed
GSPP opened this issue Jun 9, 2016 · 9 comments
Closed

Make (condition ? 1 : null) just work #11886

GSPP opened this issue Jun 9, 2016 · 9 comments

Comments

@GSPP
Copy link

GSPP commented Jun 9, 2016

The ?: operator is awkward to use when one of the branches is null and the other branch is a non-nullable value type.

var nullableInteger = condition ? 1 : null; // instead of:
var nullableInteger = condition ? 1 : default(int?); // or:
var nullableInteger = condition ? (int?)1 : null;

(Code taken from #2136 (comment)).

For some reason this pattern seems to come up frequently. Often, it's only a subexpression. For that reason a statement-based solution such as the following is not always working (and is not good style anyway):

int? nullableInteger = null;
if (condition) nullableInteger = 1;

Is there a way the C# language could be changed to make this just work?

@realbart
Copy link

realbart commented Jun 9, 2016

Even though it is technicaly correct you cannot cast from a struct to null or from a null to a struct, the c# language is already aware of Nullable. Expanding compilation of the conditional operator to be able to handle nulls and structs would not be a breaking change.

I think it should be compiled to the same code as the third line here:

const int? yesValue = 1;
const int? noValue = null;
var nullableInteger = condition ? yesValue : noValue;

I'm not sure, if these lines compile into the same code. The first one might include the cast. This may lead to unexpected results, especially when building expression trees.

var nullableInteger = condition ? (int?)1 : null;
var nullableInteger = condition ? 1 : default(int?);

@GSPP
Copy link
Author

GSPP commented Jun 9, 2016

Is there any reason we could not just add the special case that the ?: operator considers an additional implicit conversion to null? Would that break anything? Would that be unintuitive in any case? I cannot think of anything but I think the C# team is aware of this issue. There must be a reason this was not done. I'd like to encourage the team to try harder to make this work :)

@MrJul
Copy link
Contributor

MrJul commented Jun 12, 2016

See also #438

@GSPP
Copy link
Author

GSPP commented Jun 16, 2016

I just realize new [] { null, 1 } is another case where the same issue applies. The array type should be inferred to be int?. I have had this particular problem in the past.

@Richiban
Copy link

I agree: it's super confusing that the following is legal:

        public int? Demo(bool condition)
        {
            if (condition)
                return 1;
            else
                return null;            
        }

but this is not:

        public int? Demo2(bool condition)
        {
            return condition ? 1 : null;
        }

@GSPP
Copy link
Author

GSPP commented Aug 15, 2016

Something else that comes up often is this:

someCondition ? someArray : someIList

This cannot be resolved automatically. It would be convenient if the resulting type was just resolved to IList. Here's executable code to play with:

            var someCondition = true;
            var someDataSource = new string[10];
            var results = someCondition ? new string[0] : someDataSource.Where(x => true).ToList();

Is is from an actual scenario that I had. In case of someCondition I want an empty result. Otherwise I want to access a data source.

There are workarounds to make this compile but can't the language just use the common base type (here: IList<string>)?.

I don't want to specifically demonstrate a case involving lists. Clearly, the language should not special case for IList. But can't the language just use the common base type? Maybe with an extra rule to never pick object as the common base type.

@realbart
Copy link

@GSPP first of all: I HATE lists. because some people think it's a good idea to modify them, or even modifying ILists without checking if they can. Thank god Arrays implement IReadOnlyList nowadays.
For your question, right now you can conveniently:

    var someCondition = true;
    var someDataSource = new string[10];
    var results = someCondition ? new string[0] : someDataSource.Where(x => true).ToArray();

or:

    var someCondition = true;
    var someDataSource = new string[10];
    var results = someCondition ? Enumerable.Empty<string>() : someDataSource.Where(x => true);

I used to also think that C# should be able to implicitly cast to the nearest base class, but this gets a bit vague if multiple interfaces are implemented: should result be an IList or an IReadOnlyList? (one does not implement the other).

So allowing "condition ? list : array" is not as straightforward as you'd think.

@GSPP
Copy link
Author

GSPP commented Aug 15, 2016

I understand the workarounds and I'm not interested in applying them. They always are special case solutions to this more general problem.

The multiple interface argument is valid. In particular it could cause code to break when 3rd party code chooses to add interface implementations.

@gafter
Copy link
Member

gafter commented Sep 27, 2017

We are now taking language feature discussion in other repositories:

Features that are under active design or development, or which are "championed" by someone on the language design team, have already been moved either as issues or as checked-in design documents. For example, the proposal in this repo "Proposal: Partial interface implementation a.k.a. Traits" (issue 16139 and a few other issues that request the same thing) are now tracked by the language team at issue 52 in https://github.com/dotnet/csharplang/issues, and there is a draft spec at https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md and further discussion at issue 288 in https://github.com/dotnet/csharplang/issues. Prototyping of the compiler portion of language features is still tracked here; see, for example, https://github.com/dotnet/roslyn/tree/features/DefaultInterfaceImplementation and issue 17952.

In order to facilitate that transition, we have started closing language design discussions from the roslyn repo with a note briefly explaining why. When we are aware of an existing discussion for the feature already in the new repo, we are adding a link to that. But we're not adding new issues to the new repos for existing discussions in this repo that the language design team does not currently envision taking on. Our intent is to eventually close the language design issues in the Roslyn repo and encourage discussion in one of the new repos instead.

Our intent is not to shut down discussion on language design - you can still continue discussion on the closed issues if you want - but rather we would like to encourage people to move discussion to where we are more likely to be paying attention (the new repo), or to abandon discussions that are no longer of interest to you.

If you happen to notice that one of the closed issues has a relevant issue in the new repo, and we have not added a link to the new issue, we would appreciate you providing a link from the old to the new discussion. That way people who are still interested in the discussion can start paying attention to the new issue.

Also, we'd welcome any ideas you might have on how we could better manage the transition. Comments and discussion about closing and/or moving issues should be directed to #18002. Comments and discussion about this issue can take place here or on an issue in the relevant repo.


This proposal is being tracked at dotnet/csharplang#33

@gafter gafter closed this as completed Sep 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants