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

Refactor the add and remove methods in allies_controller.rb #442

Closed
andrew-schutt opened this issue May 19, 2017 · 2 comments
Closed

Refactor the add and remove methods in allies_controller.rb #442

andrew-schutt opened this issue May 19, 2017 · 2 comments
Assignees
Labels

Comments

@andrew-schutt
Copy link
Collaborator

andrew-schutt commented May 19, 2017

Both of these methods are quite long (44 lines for add and 54 lines for the remove) it makes the methods difficult to read. rubocop has a default method length of 10 lines more information about this cop here

A good plan of attack for this issue would be the following:

  1. Ensure that the tests for these two methods are complete and have full coverage
    1a. Tests can be found in the project directory structure at /spec/controllers/allies_controller_spec.rb
  2. Begin extracting methods out from the two methods. Try to keep the new methods at about 10 lines in length.
    2a. Anywhere you see duplication or queries are good for extracting into separate methods
    2b. Extracted methods can be placed inside models, controllers, or new Ruby objects entirely
  3. Rename variables capture query results that are being rerun (if possible)
  4. Rerun tests, add new tests, refactor more!

relevant RailsCast episode

@andrew-schutt andrew-schutt changed the title Refactor the add and remove methods in allies_controller.rb Refactor the add and remove methods in allies_controller.rb May 19, 2017
@julianguyen
Copy link
Member

This is a great plan! Thanks for this 😄 Will you be working on this?

@andrew-schutt
Copy link
Collaborator Author

I sure will be! I was starting some work for #14 and realized that I need to this refactor in before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants