jonleighton / focused_controller

MIT License
468 stars 27 forks source link

Why :run instead of :call #8

Closed jeremyf closed 12 years ago

jeremyf commented 12 years ago

I'm wondering if you'd considered using :call instead of :run for the method name that does the rendering?

jonleighton commented 12 years ago

@avdi asked this also.

I don't think #call makes sense. It is not a "callable thing". By which I mean I could not meaningfully substitute a proc for the controller object. The other use of #call in rubyland is for Rack apps, but again, this method doesn't take an env parameter and is not the entry point for Rack.

So basically, I think it would muddy the waters to name the method call.

avdi commented 12 years ago

I recommend @JEG2's latest Rubies in the Rough article for why #call should be the default for single-method objects: http://subinterest.com/rubies-in-the-rough/19-single-method-classes/shared/5ee6912c0307a0cd26b53032fef980491c2887c7

As he points out, any oddness in the naming is more than made up for by it's versatility. Personally, I particularly like the fact that when I am testing an object which interacts with a callable, I can simply substitute a lambda for the callable and not even bother with a stubbing library.

avdi commented 12 years ago

...the other reason I don't like #run is that to me, it actually makes even less sense than #call for a controller. I don't tell a controller to "run". I tell it to do its job... control the request. Controller#control is too redundant. Controller#handle_request might be acceptable, but it's a bit long. Some folks would say #perform, but the Ruby idiom for #perform is #call.

JEG2 commented 12 years ago

It's obvious I agree with @avdi, but I think his last comment really nailed the why in this discussion. Ruby already has a name for a single perform/run/handle() action. That name is call().

jonleighton commented 12 years ago

This feels like a bit of a bike shed issue but I'm basically persuaded, so I will reopen and implement (or wait for patches ;) On reflection I agree that the name run isn't really appropriate and call is a better choice. Thanks.

alexeymuranov commented 12 years ago

Is there a reason to use single-method objects instead of just "functions"?

jeremyf commented 12 years ago

@alexeymuranov When you mean functions do you mean anonymous functions/lambdas? Or are you meaning methods in the controller?

In reference to methods in the controller, the README addresses this…namely the single responsibility principle is often times compromised in the controller. The traditional #index action is conceptually quite different than the #destroy action. And as @jonleighton highlights, this gem is in some ways a thought experiment for others to use and experiment with. As a Rails community, we are all cognizant of the "traditional" CRUD methods in the controller. Personally, I find the controller to be a frustrating concept to both test and ultimately keep clean and focused.

In reference to lambdas, I think the structure of an Action class makes a lot of sense.

In either case, the single public method object is a potential concern…however the broad focus of a gvien resource's controller has it's own code smell.

alexeymuranov commented 12 years ago

@jeremyf, I mean that I do not understand well why controllers need to be objects (with their own data) and not simply collections of functions in the sense of functional programming. In ruby this would probably mean being modules with only module methods, used as namespaces, organized by access rights or models used. Something like:

module ShowUsersToOtherUsersController
  def self.show_all(request)
    response = Response.new
    # ...
    return response
  end

  def self.show_one(request)
    response = Response.new
    # ...
    return response
  end

  private
    def self.attributes_to_show
      [:username, :full_name]
    end

end

This is probably not a good example, and I am not proposing the change, just trying to understand what would be wrong with this approach.

UPDATE: Ok, i can partially respond to my own question: @response = Response.new can be put into initialize.

jeremyf commented 12 years ago

@alexeymuranov Interesting. I'd be curious to see this pushed out as well.

Ultimately what I find useful for focused_controller is the OO principles that it tightly cleaves to.

avdi commented 12 years ago

Because that loses the benefits of inheritance and polymorphism.