Hi,
I was thinking whether it would be more elegant for a view to call
methods on the controller rather than rely on instance variables. I''m
not really sure what the answer is so thought I would throw it out
here.
Perhaps with a basic DSL that makes it explicit what the controller is
setting, like:
class BooksController < ActionController::Base
var :book do
Book.find(params[:id])
end
end
where var:
class ApplicationController..
def self.var(name, &block)
define_method(name, block)
helper_method(name)
# Perhaps memoize too?
end
end
Does this violate the rule that the controller should be driving the
view? Any major disadvantages you can see?
Regards,
Andrew
--
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en.
Andrew France wrote:> Hi, > > I was thinking whether it would be more elegant for a view to call > methods on the controller rather than rely on instance variables.The trouble with this is that controller methods are actions, not getters.> I''m > not really sure what the answer is so thought I would throw it out > here. > Perhaps with a basic DSL that makes it explicit what the controller is > setting, like: > > class BooksController < ActionController::Base > var :book do > Book.find(params[:id]) > end > end > > where var: > > class ApplicationController.. > def self.var(name, &block) > define_method(name, block) > helper_method(name) > # Perhaps memoize too? > end > end > > Does this violate the rule that the controller should be driving the > view?Yes. The controller just passes variables to the view.> Any major disadvantages you can see?Lots. Does the view even have the controller object in scope? Should it need to? What if you want to use the same partial with different controllers? If you want something like this done right, check out Mustache.> > Regards, > AndrewBest, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.org -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On 09/02/10 01:32, Marnen Laibow-Koser wrote:> Andrew France wrote: > >> Hi, >> >> I was thinking whether it would be more elegant for a view to call >> methods on the controller rather than rely on instance variables. >> > The trouble with this is that controller methods are actions, not > getters. >It''s quite common for controllers to have methods which are not directly actions. By default they are accessible from URLs but it''s good practice to remove the default route anyway. Plus they could be set to protected (workaround probably required) or hidden with hide_action.>> I''m >> not really sure what the answer is so thought I would throw it out >> here. >> Perhaps with a basic DSL that makes it explicit what the controller is >> setting, like: >> >> class BooksController< ActionController::Base >> var :book do >> Book.find(params[:id]) >> end >> end >> >> where var: >> >> class ApplicationController.. >> def self.var(name,&block) >> define_method(name, block) >> helper_method(name) >> # Perhaps memoize too? >> end >> end >> >> Does this violate the rule that the controller should be driving the >> view? >> > Yes. The controller just passes variables to the view. > > >> Any major disadvantages you can see? >> > Lots. Does the view even have the controller object in scope? Should > it need to? What if you want to use the same partial with different > controllers? >The code above already works, if that answers your question. As for using partials, well one of reasons for using methods is that you''re making the contract between the view and the controller more explicit. You already have it with instance variables but I suppose using unset instance vars to determine logic in views is a little easier albeit less elegant. Regards, Andrew -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On Feb 8, 4:44 am, Andrew France <andrew+li...-nb9sXpffzH/10XsdtD+oqA@public.gmane.org> wrote:> Hi, > > I was thinking whether it would be more elegant for a view to call > methods on the controller rather than rely on instance variables. I''m > not really sure what the answer is so thought I would throw it out > here. > Perhaps with a basic DSL that makes it explicit what the controller is > setting, like: > > class BooksController < ActionController::Base > var :book do > Book.find(params[:id]) > end > end > > where var: > > class ApplicationController.. > def self.var(name, &block) > define_method(name, block) > helper_method(name) > # Perhaps memoize too? > end > end > > Does this violate the rule that the controller should be driving the > view? Any major disadvantages you can see?I like the idea for several reasons, not the least of which is that we use methods in partials. This approach makes it easier to move stuff around. FYI - there''s already a gem that does this with pretty much the same syntax you proposed: http://github.com/voxdolo/decent_exposure. Cheers, David> Regards, > Andrew-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
Marnen Laibow-Koser
2010-Feb-08 15:16 UTC
Re: Re: Controller methods over instance variables?
Andrew France wrote:> On 09/02/10 01:32, Marnen Laibow-Koser wrote: >> Andrew France wrote: >> >>> Hi, >>> >>> I was thinking whether it would be more elegant for a view to call >>> methods on the controller rather than rely on instance variables. >>> >> The trouble with this is that controller methods are actions, not >> getters. >> > > It''s quite common for controllers to have methods which are not directly > actions.Not in my applications. It''s inappropriate to put that much logic in the controller. The only exceptions are things like current_user, which can''t really live anywhere else, and current_object, which is simply a refactoring of controller code. Anything else belongs in a model or a helper.> By default they are accessible from URLs but it''s good practice > to remove the default route anyway. Plus they could be set to protected > (workaround probably required) or hidden with hide_action.Just don''t have them. It''s conceptually wrong for the way Rails works.> > >>> end >>> >> controllers? >> > > The code above already works, if that answers your question. > > As for using partials, well one of reasons for using methods is that > you''re making the contract between the view and the controller more > explicit.Perhaps, but it couples the view to one controller, which is wrong.> You already have it with instance variables but I suppose > using unset instance vars to determine logic in views is a little easier > albeit less elegant.Don''t do that much either. Use a helper...> > Regards, > AndrewBest, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.org -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On Feb 8, 9:16 am, Marnen Laibow-Koser <li...-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote:> Andrew France wrote: > > On 09/02/10 01:32, Marnen Laibow-Koser wrote: > >> Andrew France wrote: > > >>> Hi, > > >>> I was thinking whether it would be more elegant for a view to call > >>> methods on the controller rather than rely on instance variables. > > >> The trouble with this is that controller methods are actions, not > >> getters. > > > It''s quite common for controllers to have methods which are not directly > > actions. > > Not in my applications. It''s inappropriate to put that much logic in > the controller. > > The only exceptions are things like current_user, which can''t really > live anywhere else, and current_object, which is simply a refactoring of > controller code. Anything else belongs in a model or a helper.I completely agree with Marnen. Fat models, skinny controllers. Your example is a poor one - it''s certainly appropriate to have a @book instance variable in the BooksController. Your instinct is correct about lots of instance variables, but they should usually be replaced with model instance methods. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
On Feb 10, 12:09 am, Adam Stegman <adam.steg...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:> On Feb 8, 9:16 am, Marnen Laibow-Koser <li...-fsXkhYbjdPsEEoCn2XhGlw@public.gmane.org> wrote: > > Andrew France wrote: > > > On 09/02/10 01:32, Marnen Laibow-Koser wrote: > > >> Andrew France wrote: > > > >>> Hi, > > > >>> I was thinking whether it would be more elegant for a view to call > > >>> methods on the controller rather than rely on instance variables. > > > >> The trouble with this is that controller methods are actions, not > > >> getters. > > > > It''s quite common for controllers to have methods which are not directly > > > actions. > > > Not in my applications. It''s inappropriate to put that much logic in > > the controller. > > > The only exceptions are things like current_user, which can''t really > > live anywhere else, and current_object, which is simply a refactoring of > > controller code. Anything else belongs in a model or a helper. > > I completely agree with Marnen. Fat models, skinny controllers. > > Your example is a poor one - it''s certainly appropriate to have a > @book instance variable in the BooksController. Your instinct is > correct about lots of instance variables, but they should usually be > replaced with model instance methods.Which is a philosophy I try to subscribe to, but there are many instances where I wish to pass multiple values to a view. Perhaps because the data is not related in the database or I wish to control the dataset that is passed to the view. For example, if I have a show action for Book, as a convenience I may wish to display all the authors on the system that the user can access: def show @book = Book.find(params[:id]) @all_authors = Author.all end Is that not reasonable? It''s a contrived and simplified example of what I deal with in most apps, where I need to get data from unrelated models. Or sometimes I may create instance variables from related data (e.g. @authors @book.authors) because I wish to re-use the view in other controllers. So with my suggestion it could be written as (short block syntax): var(:book) { Book.find(params[:id]) } var(:all_authors) { Author.all } var(:authors) { book.authors } Is that better or worse? Harder to read or improves comprehension? Or has no purpose at all? The problem with instance variables is that my view is not inside the controller class, so it''s reasonable to ask if it''s appropriate for these variables to appear ''outside'' of their class. It is also easy to forget to set the variable in the controller, which is often done on purpose and checked for in view logic. But by replacing instance variables with methods you are creating a more explicit interface contract that you expect one or more controllers to adhere to. Thanks, Andrew -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.
Andrew France wrote:> On Feb 10, 12:09�am, Adam Stegman <adam.steg...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: >> > >> The trouble with this is that controller methods are actions, not >> > controller code. �Anything else belongs in a model or a helper. >> >> I completely agree with Marnen. Fat models, skinny controllers. >> >> Your example is a poor one - it''s certainly appropriate to have a >> @book instance variable in the BooksController. Your instinct is >> correct about lots of instance variables, but they should usually be >> replaced with model instance methods. > > Which is a philosophy I try to subscribe to, but there are many > instances where I wish to pass multiple values to a view. Perhaps > because the data is not related in the database or I wish to control > the dataset that is passed to the view. > > For example, if I have a show action for Book, as a convenience I may > wish to display all the authors on the system that the user can > access: > def show > @book = Book.find(params[:id]) > @all_authors = Author.all > end > > Is that not reasonable?I think it is. You''re doing simple calls to model methods and displaying the results.> It''s a contrived and simplified example of what I deal with in most > apps, where I need to get data from unrelated models. Or sometimes I > may create instance variables from related data (e.g. @authors > @book.authors) because I wish to re-use the view in other controllers.Right. If you do a lot of that, perhaps you should introduce a presenter.> > So with my suggestion it could be written as (short block syntax): > var(:book) { Book.find(params[:id]) } > var(:all_authors) { Author.all } > var(:authors) { book.authors } > > Is that better or worse? Harder to read or improves comprehension? Or > has no purpose at all?No purpose at all, I''d say. It seems pretty well equivalent to the first form> > The problem with instance variables is that my view is not inside the > controller class, so it''s reasonable to ask if it''s appropriate for > these variables to appear ''outside'' of their class.It''s the Rails way, thanks to some magic. It''s not that the variables appear outside their class; rather, the values are copied from the controller to the view.> It is also easy to > forget to set the variable in the controller, which is often done on > purpose and checked for in view logic.And the same is true with your proposal.> But by replacing instance > variables with methods you are creating a more explicit interface > contract that you expect one or more controllers to adhere to.No. The two interfaces are equivalent, and created the same contract. However, your implementation puts methods in the controller that don''t belong there. This is a bad idea.> > Thanks, > AndrewBest, -- Marnen Laibow-Koser http://www.marnen.org marnen-sbuyVjPbboAdnm+yROfE0A@public.gmane.org -- Posted via http://www.ruby-forum.com/. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group. To post to this group, send email to rubyonrails-talk-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org To unsubscribe from this group, send email to rubyonrails-talk+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-talk?hl=en.