search

dotnet / csharplang

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

Proposal: Allow ?? throw expressions in isolation #580

Closed jamesqo closed 7 years ago

jamesqo commented 7 years ago

Background

C# 7.0 lets you do this:

public Person(string firstName, string lastName)
{
    _firstName = firstName ?? throw new ArgumentNullException(nameof(firstName));
    _lastName = lastName ?? throw new ArgumentNullException(nameof(lastName));
}

However, this is illegal:

public Person(string firstName, string lastName)
{
    firstName ?? throw new ArgumentNullException(nameof(firstName));
    lastName ?? throw new ArgumentNullException(nameof(lastName));

    _firstName = firstName;
    _lastName = lastName;
}

Admittedly, the first form is usually better/terser for constructors. However, in some methods you have a bunch of business logic and the first time you use a parameter is many lines later. In those cases, if you opt to validate your parameters at the exact moment you use them with throw expressions, it can be hard to ensure that you've validated all of your parameters. The second form is better when you want to validate all of your parameters up front, and still terser than existing code of the if (firstName == null) { throw ...; } form.

Proposal

Allow the second form. Specifically, do not require expressions with ?? throw ... to be assigned to a variable.

Admittedly, I don't find expr ?? throw ... in isolation to be the most aesthetically pleasing because my mind has long associated ?? to mean a value is being replaced, rather than a side-effect being produced, so it seems a little weird at first sight. However, I think this solution (for making non-null validation shorter) is most consistent with what we've already permitted into the language.

Alternatives/Other Suggestions

public void Write(byte[] buffer, int index, int count)
{
    buffer ?? throw new ArgumentNullException(nameof(buffer));
    (index >= 0 && count >= 0) || throw new ArgumentOutOfRangeException();
    (buffer.Length - index >= count) || throw new ArgumentOutOfRangeException();
}
jveselka commented 7 years ago

I would rather see full-fledget contracts, but this is nice step forward.

DavidArno commented 7 years ago

Currently, your first example could be expressed in one of the following two ways:

public Person(string firstName, string lastName)
{
    _ = firstName ?? throw new ArgumentNullException(nameof(firstName));
    _ = lastName ?? throw new ArgumentNullException(nameof(lastName));

    _firstName = firstName;
    _lastName = lastName;
}

public Person(string firstName, string lastName)
{
    if (firstName == null) throw new ArgumentNullException(nameof(firstName));
    if (lastName == null) throw new ArgumentNullException(nameof(lastName));

    _firstName = firstName;
    _lastName = lastName;
}

Likewise your second example can be expressed as:

public void Write(byte[] buffer, int index, int count)
{
    if (buffer == null) throw new ArgumentNullException(nameof(buffer));
    if (index < 0 || count < 0) throw new ArgumentOutOfRangeException();
    if (buffer.Length - index < count) throw new ArgumentOutOfRangeException();
}

Allowing syntax that disregards the result of an expression (which would be a big step backward in the language's evolution) just to save a few characters really would not be a good thing to do in my view.

orthoxerox commented 7 years ago

Would be made possible by #538. Not sure if writing it like this is a good idea, though.

DavidArno commented 7 years ago

@orthoxerox,

How would that work? I can see how the fact that never is an implicit subtype of all types, including void would mean (expression terminating) statements could be treated as an expression. I don't see how it would allow expressions to be treated as statements. Could you give an example of how it would work, please?

orthoxerox commented 7 years ago

@DavidArno Expressions are already treated as statements. x = 1 is an expression and x = 1; is an expression statement. Right now throw is a pseudo-expression, if it was a real expression of type never you could use it in an arbitrary expression statement.

HaloFour commented 7 years ago

@orthoxerox

IIRC the problem here isn't throw. The problem is that ?? isn't permitted in a statement, only an expression. I'd suspect that if the compiler allowed ?? to be used in statements such as firstName ?? "foo"; that throw would already work in that context.

DavidArno commented 7 years ago

@orthoxerox,

Ah, I see what you were saying now. Since never is implicitly a subtype of bool, then if throw implied a never result, then it could be used in any expression. So the OP's second example could be expressed as:

public void Write(byte[] buffer, int index, int count)
{
    _ = buffer ?? throw new ArgumentNullException(nameof(buffer));
    _ = (index >= 0 && count >= 0) || throw new ArgumentOutOfRangeException();
    _ = (buffer.Length - index >= count) || throw new ArgumentOutOfRangeException();
}
lachbaer commented 7 years ago

Because _ = introduces a 'statement like expression' I would prefer, just for a clearer optical appearance a more lambda expression syntax like form:

public void Write(byte[] buffer, int index, int count)
{
    _=> buffer ?? return;
    _=> (index >= 0 && count >= 0) || return;
    _=> (buffer.Length - index >= count) || throw new ArgumentOutOfRangeException();
}

IMO, the arrow => just gives a better seperation between the left part (assignment) and the actual doing-something-statement. Of course, the arrow as 'assignment' operator is only allowed in this very discard context.

svick commented 7 years ago

@lachbaer

_=> is already valid in an expression. I would find it very confusing if it had a very different meaning in an expression statement.

lachbaer commented 7 years ago

@svick It needn't to be _=>. It is just a suggestion for a different appearance, because _= buffer ?? return; looks somehow 'cut' in the middle to me, not 'complete' enough from a visual point. I just think that it would be nice to have something lightweight that introduces an expression statement and makes them stand out a bit more, though it is not necessarily needed with _=.

jamesqo commented 7 years ago

I am reversing on this proposal. It has more thumbs-down than thumbs-up and the more I think about it, the weirder control-flow/side effects mixed with expressions seem to me. I am still standing by the proposal at https://github.com/dotnet/corefx/issues/17068#issuecomment-286550748 for terser validation, however.