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

If/else blocks should not have a negated condition in if. #34

Closed
seantanly opened this issue Jan 27, 2016 · 4 comments
Closed

If/else blocks should not have a negated condition in if. #34

seantanly opened this issue Jan 27, 2016 · 4 comments

Comments

@seantanly
Copy link

In writing if/else statements, I would normally prefer to place the short shorter block in front, and leave the larger block behind. If it's written the other way round, the reader might miss the else clause.

For example,

if ! valid(item) do
  {:error, :invalid_item}
else
  ... processing ...
  ...
  {:ok, result}
end

It is not every time we have the opportunity to rewrite the valid/1 method to invalid/1 such that there isn't a negation in the if clause. And we probably agree in not using unless/else even in such situations.

@rrrene
Copy link
Owner

rrrene commented Jan 27, 2016

I certainly would not argue against doing this. The "rule of thumb" should be: Put the positive case first.

However, your example is exactly the reason why we will have some way to disable checks on a per-case basis in Credo v0.3.0!

@seantanly
Copy link
Author

Thanks! Looking forward to Credo v0.3.0! 👍

@rrrene
Copy link
Owner

rrrene commented Feb 10, 2016

@seantanly Credo v0.3.0 now includes support for a @lint attribute. See CHANGELOG for further details.

What do you think?

@seantanly
Copy link
Author

Thanks for @lint! And together with .credo.exs gives us full ability for customization.

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

2 participants