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

Avoid confusion with class methods and protected/private modifiers #1512

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Jan 1, 2020

Working on something else, I saw the following:

protected

# ...

def self.field_type
  to_s.split("::").last.underscore
end

This code can lead to misunderstanding, as the protected modifier doesn't apply to class methods like self.field_type.

On this PR, I move two such class methods behind the modifier, near the top of the class, to avoid this confusion.

I also move three other class methods to the top of the class, as I feel class methods should go together in general, in order to clarify interfaces and avoid future instances of this anti-pattern.

@nickcharlton nickcharlton added the maintenance to keep up with changes around us label Feb 12, 2020
@pablobm pablobm force-pushed the clear-protected-confusion branch from 19ff02e to 9f465f2 Compare February 13, 2020 13:38
@pablobm pablobm merged commit 35f72ad into thoughtbot:master Feb 13, 2020
@pablobm pablobm deleted the clear-protected-confusion branch June 24, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance to keep up with changes around us
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants