amatsuda / traceroute

A Rake task gem that helps you find the unused routes and controller actions for your Rails 3+ app
MIT License
902 stars 38 forks source link

application_controller methods #2

Closed plentz closed 13 years ago

plentz commented 13 years ago

If I have public methods in the application controller, traceroute says that these methods are unreachable action methods as well. I think it should not list, since in the application controller we could have methods that must be public(at last in my understanding), like a method to handle 404's, like this one:

def page_not_found(args = nil)
  render "shared/_404", :status => 404
end

If you put it as protected/public, an UnknownAction exception could not be handled(for example).

rescue_from ActionController::UnknownAction, :with => :page_not_found

So I think it's better to just show the current action methods(or have a way to not show this ones)

amatsuda commented 13 years ago

Thanks for your feedback, but I think your example is untrue. rescue_from can actually handle private/protected methods. Please see this test case https://github.com/rails/rails/blob/master/actionpack/test/controller/rescue_test.rb#L147

IMO, all methods in ApplicationController should be privatized, and all action methods should be implemented in each concrete controller class, to make your code clean and maintainable. If you have public methods in your ApplicationController, there should be something wrong with your design or the framework.

plentz commented 13 years ago

amatsuda, I agree with you(ApplicationController should be privatized). I just created a new rails app and tested my example: it really works with private methods. It's something with my app. Closing the issue and if I find that's a case that it must be public, I reopen the issue :)