-
-
Save jcasimir/2160696 to your computer and use it in GitHub Desktop.
class ApplicationController < ActionController::Base | |
extend AttributeViewable | |
end |
module AttributeViewer | |
def attr_viewable(*names) | |
names.each do |name| | |
attr_accessor name | |
helper_method name | |
private "#{name}=".to_sym | |
end | |
end | |
end |
<table> | |
<% messages.each do |message| %> | |
<tr> | |
<td><%= message.subject %></td> | |
<td><%= message.body %></td> | |
<td><%= link_to 'Show', message %></td> | |
<td><%= link_to 'Edit', edit_message_path(message) %></td> | |
<td><%= link_to 'Destroy', message, confirm: 'Are you sure?', method: :delete %></td> | |
</tr> | |
<% end %> | |
</table> |
class MessagesController < ApplicationController | |
attr_viewable :message, :messages | |
def index | |
self.messages = Message.all | |
end | |
end |
Made the setter private in the module...
I like it.
One nice side effect is consistency with partial locals. 👍
Are at-signs really that horrible?
Lazy, and they invite unintended modification. An object should only have one reason (and one interface) to change.
Just thinking out loud here:
I like the idea of using methods over ivars. The thing I don't like is that the views have methods exposed to them that they shouldn't. For example, in this case, a view that's dealing with messages
probably will not deal with the message
method. Yet the view's context will respond to the message
method and calling that method will return a nil
. That means you won't know you inadvertently called that method until later down the road when you try to call a method on nil
.
Maybe it would be better to set a context object. The view context would always responds to a particular method, but the type of object returned by that method changes depending on the action that's called. (I guess this is called a "presenter", though I like to call it an object that's composed of things. 😉) Then if the "messages" view inadvertently calls message
on the context object, you immediately have an exception raised.
Not to mention easier testability of the composed object. :-D
@tenderlove I like how that's handled in the JS templating world, where the context is usually a javascript object. However, to apply that to this example, this.messages
would be the key pointing to a collection. How else would you handle accessing a messages
collection? context.each {..}
?
@scottburton11 I would add a messages
method to the context object. So in your view, you'd say ctx.messages.each { ... }
. The context object would be constructed in each action, like this:
class MessagesController < ApplicationController
attr_accessor :context
helper_method :context
IndexContainer = Struct.new(:messages)
ShowContainer = Struct.new(:message)
def index
self.context = IndexContainer.new(Message.all)
end
def show
self.context = ShowContainer.new(Message.find(params[:id]))
end
end
Then if you accidentally call context.message
in the view of your index, you'll get an exception. Or if you call context.messages
from your show view, you get an exception.
Ah, I see now.
How hard would it be to mixin the context methods so messages
will work without the context.
? I cannot currently think of an easy way to achieve that.
@myobie you could mix in a module on each request. So something like:
class MessagesController < ApplicationController
def index
extend Module.new {
attr_accessor :messages
}
self.messages = IndexContainer.new(Message.all)
end
end
But I strongly advise against doing that. It breaks method caches which will slow method lookup on every request, and couples your code with the fact that the controller is a new instance on every request.
Usage of a context (or container, or presenter, or whatever you want to call it) object allows you to:
- test behavior of the container outside of your controller (you don't need to write a functional test to test the Container)
- decouple the view from the controller (you could test the view without instantiating a controller)
- get NoMethodErrors raised exactly at the point where the error occurred (rather than waiting for the
nil
to be used)
Thanks @tenderlove, fixed.
@scottburton11 I agree, though this currently exposes the setters publicly. I'm going to play with it for a few more minutes...