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

"unnecessary negation" (if-not-else) is a bit annoying #859

Closed
birkenfeld opened this issue Apr 16, 2016 · 4 comments
Closed

"unnecessary negation" (if-not-else) is a bit annoying #859

birkenfeld opened this issue Apr 16, 2016 · 4 comments

Comments

@birkenfeld
Copy link
Contributor

Suppose the following case:

        if some_error_condition {
            Err(some_error)
        } else if some_other_error_condition {
            Err(some_other_error)
        } else {
            Ok(result)
        }

If some_other_error_condition happens to be a != b, the lint will trigger. However, I don't want to move the Ok case inbetween the Err cases.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 16, 2016

We could treat if elseif else chains as ordered and not lint.

@llogiq
Copy link
Contributor

llogiq commented Apr 16, 2016

So I see two possibilities:

  • make the lint allow by default. It does generate a lot of reports and is probably too opinionated
  • have the lint bail on encountering else ifs

Possibly both.

@mcarton
Copy link
Member

mcarton commented Apr 16, 2016

make the lint allow by default. It does generate a lot of reports and is probably too opinionated

👍 I’ve found that lint to be rather annoying myself.

mcarton added a commit that referenced this issue Apr 17, 2016
Make if_not_else lint Allow by default (fixes #859)
@GrantGryczan
Copy link

GrantGryczan commented Jan 13, 2024

For anyone stumbling upon this in the future by browsing related issues to lints (as I am), this is how I personally would write this example, rather than considering it an exception to the lint:

        if some_error_condition {
            return Err(some_error);
        }

        if some_other_error_condition {
            return Err(some_other_error);
        }

        Ok(result)

Independently of whether I use this lint, I usually use early returns/breaks for conditions that branch away from the code's happy path, as it simplifies control flow in general and reduces nesting of the happy path, giving me less to keep in my head as I read through the happy path of a function. This pattern is known as guard clauses, and here's a good video about it.

This is just my preference, but figured it might be worth noting for the record. It's also worth noting this advice doesn't apply if the branch isn't meant to return (unless it happens to be practical to extract the if statements to their own function where they can be made to return).

#7895 covers this whole issue anyway.

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

5 participants