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

Application controller / application.html.erb refactoring thoughts #84

Closed
fpcyan opened this issue Dec 7, 2015 · 2 comments
Closed

Comments

@fpcyan
Copy link
Contributor

fpcyan commented Dec 7, 2015

Some principles when writing code are the Law of Demeter and SOLID. If you're interested, I'd suggest reading the blurbs about them.

Basically, programming lives by the concept that no single function or class should know, or do, too much. At the moment, the taxonomies function violates that bit.

Similarly, the application.html.erb has many long conditional chains to determine what to render.

We can actually refactor both of these things so that 1) the controllers tell the views what title and contents to render, and 2) the models take care of their pluralization (which, I think though I'm not sure, is the main function of the taxonomies methods).

Anyway, I wanted to log an issue while I tinkered, so if there needs to be conversation about it, there can be.

@fpcyan
Copy link
Contributor Author

fpcyan commented Dec 7, 2015

Okay, so I began refactoring the application html and application controller, but I ran into some trouble in that the "fetch commits" method is used for multiple different things. This led me down the rabbit hole, and now I'm drawing up model-level assocations that are missing.
I think the easiest way to refactor the codebase would be the following:

  • add ActiveRecord associations between models
  • move logic for rendering from views to their controllers, where possible
  • fetch data needed for the views through for the controllers through associations
  • condense views logic where needed

The benefit of this is that it will actually reduce the complexity of the codebase, and allow for new featured to be plugged in easier. Also, if you want to change a feature, it will require you to update code in far fewer places (for example, if you wanted to change how you fetch from fetch_taxonomies, you might have to change logic in 9 files right now).

Also the nice thing about this is that it won't break anything to do any single step. If somebody refactors the db and model associations, it shouldn't affect the controller or views. That way all changes are incremental, and it's not one huge project to do it all. :)

Edit: Anyway, the way I got here is that "fetch_taxonomies" is actually just a replacement for things that ActiveRecord associations do.

@fpcyan
Copy link
Contributor Author

fpcyan commented Dec 7, 2015

I drew a picture. It's a little overwhelming at first so I tried to break it up using whitespace. Are other users capable of commenting on anything besides strategies? I couldn't find anywhere else on the site.
untitled 1

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