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

Support .nil? in DSL #93

Open
pelletencate opened this issue May 15, 2018 · 4 comments
Open

Support .nil? in DSL #93

pelletencate opened this issue May 15, 2018 · 4 comments

Comments

@pelletencate
Copy link

Right now, in order to check if something IS NULL, I have to use field == nil in the DSL. Wouldn't it be more ruby-esque to be able to say field.nil? instead?

@rzane
Copy link
Owner

rzane commented May 15, 2018

Hmm... sort of, but this feels pretty like it would cause unexpected problems that would be really hard to figure out what went wrong. Plus, then we'd have to support !field.nil? to produce an IS NOT NULL query.

I would rather add is_null(field) and is_not_null(field)

@chewi
Copy link
Contributor

chewi commented Jun 5, 2018

One problem with field == nil is that RuboCop keeps trying to replace it with .nil? !

@pelletencate
Copy link
Author

pelletencate commented Sep 23, 2018

Yeah, the initial reason why I proposed this change was Rubocop complaining. I feel kinda stupid not thinking about !field.nil?, that's a more legit concern than whatever I have.

tbh, is_null or is_not_null, while it would please rubocop, would in my opinion be a step in the wrong direction. A better thing would be to have @bbatsov et al join in, to think about the possibility to provide a custom rubocop plug with your gem, or – more generally – if there's anything that would help accommodate cases (a/o DSL) where the == operator gets overloaded.

Besides, Rubocop also complains about == 0. And making an is_zero?(field) and is_not_zero?(field), would be even more unnecessary.

@bbatsov
Copy link

bbatsov commented Sep 23, 2018

One problem with field == nil is that RuboCop keeps trying to replace it with .nil? !

That's configurable. :) Obviously a static analyzer can't really figure out that something might be a special DSL. I think in such situations you can just exclude the DSL files from the checks and be done with it.

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

4 participants