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

Cop idea: numbered variable names (user_1 vs user1) #3167

Closed
mockdeep opened this issue May 26, 2016 · 7 comments
Closed

Cop idea: numbered variable names (user_1 vs user1) #3167

mockdeep opened this issue May 26, 2016 · 7 comments

Comments

@mockdeep
Copy link
Contributor

We've got a lot of places in our code where we use a couple of records, something like:

user_1 = blah
user_2 = bloo

Sometimes people number them instead, user1 and user2. Would be nice to have a rule to enforce consistent numbering of variable names, so always with _ or always without.

@Drenmi
Copy link
Collaborator

Drenmi commented Jul 3, 2016

Would it also be useful to have a configuration option to disallow numbered variables? I can see how some teams might want this to encourage better design (i.e. using a data structure like Array) and naming conventions.

@mockdeep
Copy link
Contributor Author

mockdeep commented Jul 3, 2016

I can't say that it would be something that we would use. We do a lot of testing around things like one-off scripts and database scopes to ensure that only specific records are returned or modified. So something like:

user_1 = create(:user, ...)
user_2 = create(:user, ...)
expect do
  SomeThing.call
end.to change { user_1.reload.some_attribute }
  .and not_change { user_2.reload.some_attribute }

@mikegee
Copy link
Contributor

mikegee commented Jul 4, 2016

I agree with both of you. I think this new cop would have some value and disallowing numbered variables might be the best default config.

@Drenmi
Copy link
Collaborator

Drenmi commented Jul 4, 2016

There is, of course, a small chance for false positives. Consider, for example, a method call base64 within a class, which would get flagged.

@mikegee
Copy link
Contributor

mikegee commented Jul 4, 2016

What if the cop specifically looked for more than one local variable with incrementing suffix numbers? Perhaps starting at 0 or 1?

I think that would catch the problem and not miss any violations.

@mockdeep
Copy link
Contributor Author

mockdeep commented Jul 4, 2016

It almost sounds like there are multiple rules being discussed here:

  1. (my suggestion) variable names with a number should all be consistently name_1 or name1
  2. variable names should not be named with a number
  3. variable names with numbers should be sequential starting from 1 (or 0)
  4. (another I just thought of) variable names shouldn't have a number if there's only one of them

@Drenmi
Copy link
Collaborator

Drenmi commented Jul 5, 2016

@mockdeep: These would almost certainly go in the same cop (with configuration options), which is why we're discussing them all in the same thread. 😀 Checking adjacent variables is a bit more involved, but might be possible if the rules are well defined.

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

4 participants