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: if and return in one statement #1397

Closed
ghost opened this issue Mar 19, 2018 · 13 comments
Closed

Proposal: if and return in one statement #1397

ghost opened this issue Mar 19, 2018 · 13 comments

Comments

@ghost
Copy link

ghost commented Mar 19, 2018

Hi,

in many cases, especially in parameter checks, I have to write:

bool SomeMethod(int a, int b)
{
    if(CheckMethod(a)) return false;
    if(ValidateSomething(b)) return false;

    // [there is other actions here]
    return true;
}

bool AnotherMethod(int a)
{
    if(CheckMethod(a)) return false;
}

CheckMethod and ValidateSomething are methods that usually have many lines of code and are used in other methods, especially in MVC where various actions has validation steps or returns of different views/results.

My proposal is reduce the if-return statement in one statement like:


bool SomeMethod(int a)
{
    check CheckMethod(a); // if(CheckMethod(a)) return default(bool);

    // do another things
    return true;
}

object AnotherMethod(int a)
{
    check CheckMethod(a) != null; // if(CheckMethod(a) !== null) return default(object)

   // do another things
    return new object();
}

What you think about that? is hard to do... c# itself has a better approach, or this proposal has simple a bad idea?

Thanks

@svick
Copy link
Contributor

svick commented Mar 19, 2018

First, your solution does not seem to match the problem: your original code has e.g. return true;, when the if-return code seems to do return a; (which does not even match the return type of the method).

Second, you yourself seem to be unclear about which expression is tested and which is returned (as seen in the ValidateSomething example). What makes you think it will be clear to people reading such code? Especially for code like if-return CheckMethod(a) != null;, what exactly are the rules about what is tested and what is returned?

@ghost
Copy link
Author

ghost commented Mar 19, 2018

Sorry about wrong return types, I rewrite code many times.

Yes, is only ideas, but my proposal is not about the solution, but if is possible to reduce if and return statements in only one.
the idea to people when see is: "the code stop here and return if this statement is true" and don't need to write a full if statement with only return inside it like this:

bool SomeMethod()
{
    if(!SomethingWrongWithParameter(a))
    {
        return default(bool); // example only
    }
}

and use something like:

check-return SomethingWrongWithParameter(a);

@jnm2
Copy link
Contributor

jnm2 commented Mar 19, 2018

Conditionally returning default is not something I seem to end up doing much of.

See #325, #453 and #724 for existing discussion.

@jveselka
Copy link

Seems a lot like use case for code contracts #105

@Unknown6656
Copy link
Contributor

What speaks against using a combination of boolean operators and/or ternary ifs?
e.g.

bool SomeMethod(int a, int b)
{
    return CheckMethod(a)
    	&& ValidateSomething(b)
    	&& ... 
    	&& ... ;
}

@theunrepentantgeek
Copy link

Ruby already has a well established syntax for this. If memory serves, it looks like this:

bool SomeMethod(int a, int b)
{
    return false if (CheckMethod(a));
    return false if (ValidateSomething(b));

    // [there are other actions here]
    return true;
}

bool AnotherMethod(int a)
{
    return false if (CheckMethod(a));
}

This Ruby style syntax is more general purpose than your proposal - it can be used in more different places than just for guard clauses.

The idea of adding this syntax has already been discussed several times (it keeps coming up in the comments on other proposals) and it's unlikely to happen.

CheckMethod and ValidateSomething are methods that usually have many lines of code and are used in other methods, especially in MVC where various actions has validation steps or returns of different views/results.

If I saw someone write this on a code review, I'd be encouraging the author to refactor their code. To me this sounds as though there's an essential domain object begging to be identified and turned into a real class. #ymmv

@Thaina
Copy link

Thaina commented Apr 2, 2018

Also #138 dotnet/roslyn#11562

@Pzixel
Copy link

Pzixel commented May 10, 2018

@leonardobos what do you want is called macro. For example, in Rust:

macro_rules! try {
    ($expr:expr) => (match $expr {
        $crate::result::Result::Ok(val) => val,
        $crate::result::Result::Err(err) => {
            return $crate::result::Result::Err($crate::convert::From::from(err))
        }
    });
    ($expr:expr,) => (try!($expr));
}

that allows you to write:

fn foo() -> Result<bool> {
   try!(bar(a));
   try!(baz(b));
   return true;
}

which expands into:

fn foo() -> Result<bool> {
   match bar(a) {
      Ok(()) => (),
      Err(e) => return Err(From::from(e));
   }
   match baz(b) {
      Ok(()) => (),
      Err(e) => return Err(From::from(e));
   }
   return true;
}

But C# doesn't have macros so it's not currently doable.

@Pzixel
Copy link

Pzixel commented May 10, 2018

@theunrepentantgeek

Ruby already has a well established syntax for this. If memory serves, it looks like this

Ruby just places a condition after if body, it does nothing like OP is asking.

@Clockwork-Muse
Copy link

@Pzixel - in Rust you're supposed to use the ? operator now (but this doesn't materially change the results).

@Pzixel
Copy link

Pzixel commented May 10, 2018

@Clockwork-Muse I intendenly didn't use question mark operator because it's a compiler builtin (which OP is asking for) while it can be easily done with macro system (that's what I wanted to show).

@AustinBryan
Copy link

What if I wanted check CheckMethod(a); to return true? Or 5? Or anything meaningful?

@ghost
Copy link
Author

ghost commented Jun 8, 2018

Based on comments and down votes I think that is a bad idea

@ghost ghost closed this as completed Jun 8, 2018
This issue was closed.
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

9 participants