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# #8181

Closed
Eyas opened this issue Jan 26, 2016 · 16 comments
Closed

Proposal: Guard statement in C# #8181

Eyas opened this issue Jan 26, 2016 · 16 comments

Comments

@Eyas
Copy link
Contributor

Eyas commented 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
Copy link
Contributor

alrz commented Jan 26, 2016

Also check out #119 and #6400.

@Eyas
Copy link
Contributor Author

Eyas commented 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
Copy link

mburbea commented 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
Copy link
Contributor Author

Eyas commented 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
Copy link

@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
Copy link

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
Copy link
Contributor Author

Eyas commented 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
Copy link

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

@Eyas Eyas changed the title Guard statement in C# Proposal: Guard statement in C# Jan 28, 2016
@Richiban
Copy link

Richiban commented Feb 8, 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
Copy link

DerpMcDerp commented 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
Copy link

@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
Copy link

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
Copy link

@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
Copy link
Member

gafter commented 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

@gafter
Copy link
Member

gafter commented Apr 28, 2017

Issue moved to dotnet/csharplang #511 via ZenHub

@gafter gafter closed this as completed Apr 28, 2017
@breyed
Copy link

breyed commented Jun 21, 2020

For those whose searching lands you here, the canonical issue for this is dotnet/csharplang#138.

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

9 participants