Hi:
I have two controller methods, new and edit. They use the same form,
largely because they must.
I have the following code in both methods:
@bb_tasks = BbTask.find(:all)
@projects = Project.find(:all)
@clients = Client.find(:all)
@tasks = Task.find(:all)
The form needs those objects. But I can’t help feeling that I should
have another way to do this so I avoid aggravating the DRY police.
Far from the desiccated code I wish to write, I feel it is positively
soggy.
Any suggestions?
bruce
On Tue, Dec 13, 2005 at 09:07:29PM -0700, Bruce B. wrote:
The form needs those objects. But I can’t help feeling that I should
have another way to do this so I avoid aggravating the DRY police.
Far from the desiccated code I wish to write, I feel it is positively
soggy.
Any suggestions?
Use a conditional before filter:
before_filter :fetch_collections, :only => [:new, :edit]
…
private
def fetch_collections
@bb_tasks = BbTask.find(:all)
@projects = Project.find(:all)
@clients = Client.find(:all)
@tasks = Task.find(:all)
end
marcel
On 12/14/05, Bruce B. [email protected] wrote:
Hi:
I have two controller methods, new and edit. They use the same form,
largely because they must.
I have the following code in both methods:
Why cant you use the render :partial => ‘form’ …and the _form.rhtml has
all the fields needed.Sounds too simple…or am i missing something?
Marcel:
You are a thing of beauty. Thank you. That is what I needed. I had
heard of before_filter but had not made the connection.
You are a desiccant, sir!
bruce
desiccant |Ë?desikÉ?nt| |Ë?dÉ?sÉ?kÉ?nt| |Ë?dÉ?sɪk(É?)nt|
noun
a hygroscopic substance used as a drying agent.
A compliment, I’m certain!
And Marcel, that is a nice usage that will improve some of
my code as well. Thanks.
–
– Tom M.
On 12/13/05, Marcel Molina Jr. [email protected] wrote:
marcel
Marcel Molina Jr. [email protected]
Rails mailing list
[email protected]
http://lists.rubyonrails.org/mailman/listinfo/rails
I’m curious, why not just call fetch_collections at the beginning of
the method? For me that would just make more sense because I can look
at the method and know exactly what’s happening, whereas I might
forget to check to see if there’s a before_filter. I’d like to avoid
any confusion at all.
Also, I just wanted to know if there’s any difference at all between
using before_filter and just calling the method at the beginning.
Probably not, but just wanted to be sure.
Pat
ok…i guess i misunderstood the question…this filter thing is actually
really useful.never had used it before.
Pat M. wrote:
I’m curious, why not just call fetch_collections at the beginning of
the method? For me that would just make more sense because I can look
at the method and know exactly what’s happening, whereas I might
forget to check to see if there’s a before_filter. I’d like to avoid
any confusion at all.
If you only call the method one time then it makes sense to add that
method call at the top of your
action method. The OP was asking how-to makes things in a more DRY
manner. Marcel’s suggestion does
this because the OP is now shown a better way to write code for reuse.
Also, I just wanted to know if there’s any difference at all between
using before_filter and just calling the method at the beginning.
Probably not, but just wanted to be sure.
before_filters are executed before your action method is called. If a
before_filter returns false it
will not continue on to call the action method. This can be ideal for
any precondition to an action.
A good example is user authorization. You can enforce users are
authorized before accessing
certain actions, and you can do it on one line! This makes it less
error-prone then adding a call to
“authorize” at the beginning of every action, because you’re reducing
the # of repeated lines you
have to maintain.
Would you prefer?
before_filter :authorize, :only=>[ :status, :admin ]
Or…
def status
if authorize
…
else
…
end
end
def admin
if authorize
…
else
.,.
end
end
I prefer the first…
Zach
Jamie M. wrote:
…
end
def status
return unless authorize
…
end
def admin
return unless authorize
…
end
Although your examples shows more concise way to duplicate more code in
a on-method basis, it is
code duplication nonetheless, and is more error-prone to having an
update being missed since you
have to search multiple spots of code if you ever needed to change the
authorize call.
I know you prefer the filter (since you mentioned it), so this is really
to just further mention
that if code is concise, and duplicated… it’s still duplicated, it’s
best to avoid when possible.
Zach
On Wed, 2005-12-14 at 05:27 -0500, zdennis wrote:
...
end
end
def admin
if authorize
…
else
.,.
end
end
For the second one, you probably don’t need the else clause - if the
authorize method is the same for both cases, then it should be in charge
of rendering a ‘please log in’ page or whatnot.
That said, this one isn’t too bad, but I do prefer the filter
personally:
def status
return unless authorize
…
end
def admin
return unless authorize
…
end
On 12/15/05, zdennis [email protected] wrote:
if authorize
.,.
Although your examples shows more concise way to duplicate more code in a on-method basis, it is
code duplication nonetheless, and is more error-prone to having an update being missed since you
have to search multiple spots of code if you ever needed to change the authorize call.
I know you prefer the filter (since you mentioned it), so this is really to just further mention
that if code is concise, and duplicated… it’s still duplicated, it’s best to avoid when possible.
One thing that you can do that will eliminate all duplication is to
put the ‘before_filter :authorize’ call in your application controller
(works if the protected actions are the same in each controlller).
Original Poster (I presume)
–
Alex
Alex:
Thanks. That makes sense. Funny thing a brain, I just could not
derive that from the acronym. Now you say it, it seems painfully
obvious. Thanks, my curiosity is assuaged, I can now continue with a
normal life.
bruce