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

Proposal: Guard statement in C# #511

Closed
gafter opened this issue Apr 28, 2017 · 7 comments
Closed

Proposal: Guard statement in C# #511

gafter opened this issue Apr 28, 2017 · 7 comments

Comments

@gafter
Copy link
Member

gafter commented Apr 28, 2017

@Eyas commented on Tue Jan 26 2016

Background

Swift has a guard statement that can be very valuable in writing code that is easy to reason about. See Swift Guard Statement - why you should use it.

Problem

If we have a function Foo that takes an integer that must be positive (> 0), we can write that as:

Bar Foo_alt1(int b) {
    if (b > 0) {
        // all of our code is surrounded by checks for the condition.
        // special handling of condition is far away from the statement of what the condition is
        // extra indent, extra nesting, etc.
    } else {
        throw new IllegalArgumentException(nameof(b));
    }
}

Bar Foo_alt2(int b) {
    // 'syntactic sugar' disadvantage is that now we check for the "negative"
    // condition instead of the positive. this might be natural in cases of null checking or
    // other simple checks, but can become burdensome in complex cases of many comparisons
    // and logical operators.
    if (b <= 0) {
        throw new IllegalArgumentException(nameof(b));
    } else {
        // rest of code to handle the normal case is here.
        // 'excessive' nesting and indentation
        return foo;
    }
}

Bar Foo_alt3(int b) {
    // 'syntactic sugar' disadvantage is that now we check for the "negative"
    // condition instead of the positive. this might be natural in cases of null checking or
    // other simple checks, but can become burdensome in complex cases of many comparisons
    // and logical operators.
    if (b <= 0) {
        throw new IllegalArgumentException(nameof(b));
    }

    // rest of code to handle the normal case is here.
    // we choose not to resort to nesting, but in doing so, we lose an important compile-time check:
    // a programmer error might result in the if-block above not returning or throwing, thus
    // causing the control flow to drop off into the "normal" code
    return foo;
}

Proposed Solution: guard statements

Bar Foo(int b) {
    // state that b must be a positive integer
    guard (b > 0) else {
        throw new IllegalArgumentException(nameof(b));
    }
    // compiler verifies that the guard-block throws or returns, thus
    // guaranteeing that code here will satisfy the guard condition.
    // ... do something with b without worrying about edge cases
}

Benefits

The guard statement introduces two benefits over if-based guards:

  1. Syntactic sugar of declaring the positive condition we would like to meet after the guard, rather than the negatives
  2. Compile-time checking of the fact that the guard clause ends control flow by returning or throwing
  3. One less indent :)

Grammar

guard-statement
    : 'guard' ( boolean-expression ) 'else' embedded-statement
    ;

@alrz commented on Fri May 20 2016

Also check out #119 and #6400.


@Eyas commented on Tue Jan 26 2016

@alrz right. unless is what its called in Ruby, Swift is calling it guard. I prefer guard because it relates to the guard refactoring pattern.

While I like contract I think this is quite different from #119: the guard block could result in an exception thrown for an invalid argument, but could also include non-errored handling of edge cases. E.g.:

int fib(int n) {
    guard (n > 1) else return 1;
    return fib(n - 1) + fib(n - 2);
}

The else clause in #6400 is very close to what we want, but:

  1. We want to check on boolean-expression rather than pattern.
  2. The conditions we check for can span multiple variables and assignments, e.g.:
void Configure(Configuration c) {
    guard (URLExists(c.URL) or FileExists(c.File)) else {
        Console.WriteLine("Supplied configuration does not contain a valid file or URL.");
        return;
    }
    // ...
}

@mburbea commented on Tue Jan 26 2016

And what's wrong with

if (!(b > 0)){
      throw new InvalidArgumentException(...)
}

It handles your case. I don't find that unless or guard Adds a lot of value to the language. It adds another way to do the exact same thing, and then developers will have to learn new syntax, and have another form to worry about when reading code.


@Eyas commented on Tue Jan 26 2016

@mburbea This is what I explained in the "Problem" section, the main issue with the if (!(...)) { is that the compiler doesn't guarantee that the embedded statement after the if-guard terminates. So I can do something like:

if (b == null) {
   // print a complicated error
   // forget to return or throw
}

foo = b.bar(); // <-- control drops here in the b is null case and we get an error

So yes, benefits are:

  1. Checking for the positive condition (guard (b > 0) else ...) is "pretty" syntactic sugar
  2. Compiler verifies that the else clause will terminate, so that code after the guard clause provably runs only when the guard conditions are true.

@HaloFour commented on Tue Jan 26 2016

@Eyas

#6400 does handle that, else must throw or return.

Otherwise I don't see anything particularly wrong with it but I'm not sure that it's particularly necessary. I want to dislike it more just because Swift has it, but meh.


@HaloFour commented on Tue Jan 26 2016

I will say that the one reason that I dislike this and #119 is that it doesn't offer any proposal for metadata attached to the method to declare such constraints. Handling illegal arguments is one thing, preventing them from happening is significantly more.


@Eyas commented on Tue Jan 26 2016

@HaloFour See my examples in the comment above:

  1. the guard clause does not constrain parameters or variables passed, but rather provides a guarantee on these variables after the statement. The else clause can simply return (see fib example). This is valuable as-is, and is orthogonal to annotating the "type" of a function or scope.
  2. the guard clause is different than let in that:
    • the condition is a boolean expression, not a pattern
    • the condition could be a global thing, or an and of multiple values, or a more complex condition, rather than a condition related to a single variable or assignment. See the Configure example.

@HaloFour commented on Tue Jan 26 2016

@Eyas You're right. I think after the defer proposal I was looking for a fight. 😉


@Richiban commented on Mon Feb 08 2016

I still think that the requirements this proposal seeks to address would be supported by pattern matching -- I think it would even look fairly similar. Consider this pseudocode:

public int F(int? arg)
{
    match (arg)
    {
        null | x when x < 0 =>
        {
            throw new ArgumentException("arg cannot be null or negative");
        }
        i =>
        {
            // Do something with variable 'i' which is now a regular int
        }
    }
}

You get all the requested features above:

  • the guard appears first in the method
  • the body of the guard must return or throw
  • you get the opportunity to match on the variable given your guards passed
  • it is possible to match on anything, not just the parameters of your method

@DerpMcDerp commented on Wed May 18 2016

The big problem with guard statements (and pattern matching in general) is that the boolean test isn't able to be abstractable out e.g.

Bar Foo(int b) {
    guard (b > 0) else {
        throw new IllegalArgumentException(nameof(b));
    }
    body;
}

Forcing you to repeat the guard (b > 0) else throw blah line everywhere. It would be better if C# had an erasable psuedo-type that combines a type and a predicate in a single place once and for all, e.g. if something like this:

[Message("Number isn't positive")]
guard Positive<T>(T value) {
    return value > 0;
}

Bar Foo(Positive<int> b) {
    int c = b;
    if (c is Positive<int>) print("asdf");
    ...
}

was syntax sugar for:

Bar Foo(int b) {
    if (!(b > 0)) throw ArgumentException("Number isn't positive", nameof(b));
    int c = b;
    if (c is int && ((int)c > 0)) print("asdf");
    ...
}

@HaloFour commented on Wed May 18 2016

@DerpMcDerp

I don't know what that has to do with guard per se, nor what guard has to do with pattern matching aside the potential for patterns to be used in arbitrary conditional expressions. If you want the reusability you can just call a helper function:

public static void GuardIsPositive(int param, string name) {
    guard (x > 0) else throw new ArgumentException("Number isn't positive.", name);
}

Bar Foo(int b) {
    GuardIsPositive(b, nameof(b));
}

That seems significantly cleaner than trying to hack something into the type system.

As for reusability in pattern matching, that sounds like it would be active patterns.


@DerpMcDerp commented on Wed May 18 2016

There are 2 annoyances with the helper function approach that make it less cleaner than hacking it into the type system for common places in code:

  • it requires you to repeat variable names in places, e.g.
Bar Foo(GuardIsPositive b) {
}

// vs

Bar Foo(int b) {
    GuardIsPositive(b, nameof(b));
}
  • It requires 2 helper functions for boolean tests and exceptions rather than 1 function, e.g.
[Message("Number isn't positive.")]
guard positive(int value) {
    return value > 0;
}

Bar Foo(positive b) {
    object c = b;
    if (c is positive) print("yay");
}

// vs

public static void IsPositive(int value) {
    return value > 0;
}

public static void GuardIsPositive(int value, string name) {
    if (!IsPositive(value)) else new ArgumentException("Number isn't positive.", name);
}

Bar Foo(int b) {
    GuardIsPositive(b, nameof(b));
    object c = b;
    if (c is int and IsPositive((int)c)) print("yay");
}

@HaloFour commented on Wed May 18 2016

@DerpMcDerp It's less code, but that doesn't make it cleaner. It hides much of what it does through syntactic voodoo. It blends the concerns of normal conditions and validation. It hijacks the type system in a completely unprecedented way and changes the public contract of methods using them. As "erased" it can't be reused, requiring all of these common "guards" to be constantly reimplemented over and over again.

If we're going to go down the path of declarative validation I'd rather explore AOP options through code generators. That or #119.


@gafter commented on Wed May 25 2016

What guard has to do with pattern-matching is that pattern variables introduced in the expression of a guard statement would be in scope in the enclosing block. See #11562

@yaakov-h
Copy link
Member

yaakov-h commented Apr 30, 2017

The proposal above says the guard expression would be a boolean-expression. I'm not familiar with the intricate grammar of C# but could we make this allow pattern matching expressions, to enable code such as the following:

public void Foo(object o)
{
    guard (o is Bar b) else throw new ArgumentException(nameof(o));

    // do something with `b`...
}

@gafter
Copy link
Member Author

gafter commented Apr 30, 2017

You can already write

public void Foo(object o)
{
    if (o is Bar b) {} else throw new ArgumentException(nameof(o));

    // do something with `b`...
}

Not sure how adding a new keyword and statement form makes this any better.

@MovGP0
Copy link

MovGP0 commented May 1, 2017

using a ! might be clearer than using the {} block and the else keyword:

public void Foo(object o)
{
    if (!(o is Bar b)) throw new ArgumentException(nameof(o));

    // do something with `b`...
}

having a guard that guarantees to either return or throw seems to be a good idea. However, we could also solve this with a contract:

public void Foo(object o)
{
    Contract.Requires(o is Bar b, else: () => throw new ArgumentException(nameof(o)));
    // do something with `b`...
}

and even cleaner with #119 / #105:

public void Foo(object o)
   requires o is Bar b else throw new ArgumentException(nameof(o)))
{
    // do something with `b`...
}

I doubt that there is much use for a guard once there are contracts, so I would postpone this feature.

@gulshan
Copy link

gulshan commented May 1, 2017

The proposal without the comments was already imported in #138

@gafter
Copy link
Member Author

gafter commented May 2, 2017

OK, closing as a dup of #138.

@gafter gafter closed this as completed May 2, 2017
@FreeRadical7
Copy link

Seems like it would make more since to just create a static Guard class like so...

`
public static class Guard
{
[DebuggerHidden]
public static T GuardNull(T value, string argument)
{
if (value == null)
{
throw new ArgumentNullException(argument);
}

  return value;

}
}
`

Then you can use it like so...

public Foo(object obj) { this.Object = Guard.GuardNull(obj, nameof(obj)); }

You don't even have to use the return value of course. The issue I guess is deciding if you want to throw an exception or do some other kind of error handling. If your pattern is changing that much, then what's the point of something baked into the language?

@gafter
Copy link
Member Author

gafter commented Mar 17, 2019

@FreeRadical7 This issue was closed long ago.

@dotnet dotnet locked as resolved and limited conversation to collaborators Mar 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants