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
Vivek Krishna
2005-Dec-14 04:17 UTC
Re: I am breaking the DRY principle - could I avoid it?
On 12/14/05, Bruce Balmer <brucebalmer-ee4meeAH724@public.gmane.org> 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 hasall the fields needed.Sounds too simple..or am i missing something? _______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
Marcel Molina Jr.
2005-Dec-14 04:22 UTC
Re: I am breaking the DRY principle - could I avoid it?
On Tue, Dec 13, 2005 at 09:07:29PM -0700, Bruce Balmer wrote:> 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?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 -- Marcel Molina Jr. <marcel-WRrfy3IlpWYdnm+yROfE0A@public.gmane.org>
Bruce Balmer
2005-Dec-14 06:29 UTC
Re: I am breaking the DRY principle - could I avoid it?
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 On 13-Dec-05, at 9:22 PM, Marcel Molina Jr. wrote:> On Tue, Dec 13, 2005 at 09:07:29PM -0700, Bruce Balmer wrote: >> 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? > > 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 > -- > Marcel Molina Jr. <marcel-WRrfy3IlpWYdnm+yROfE0A@public.gmane.org> > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails
> 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 Mornini On Dec 14, 2005, at 1:29 AM, Bruce Balmer wrote:> 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!
On 12/13/05, Marcel Molina Jr. <marcel-WRrfy3IlpWYdnm+yROfE0A@public.gmane.org> wrote:> On Tue, Dec 13, 2005 at 09:07:29PM -0700, Bruce Balmer wrote: > > 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? > > 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 > -- > Marcel Molina Jr. <marcel-WRrfy3IlpWYdnm+yROfE0A@public.gmane.org> > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > 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
Vivek Krishna
2005-Dec-14 08:48 UTC
Re: I am breaking the DRY principle - could I avoid it?
ok..i guess i misunderstood the question..this filter thing is actually really useful.never had used it before. On 12/14/05, Bruce Balmer <brucebalmer-ee4meeAH724@public.gmane.org> wrote:> > 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 > > > On 13-Dec-05, at 9:22 PM, Marcel Molina Jr. wrote: > > > On Tue, Dec 13, 2005 at 09:07:29PM -0700, Bruce Balmer wrote: > >> 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? > > > > 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 > > -- > > Marcel Molina Jr. <marcel-WRrfy3IlpWYdnm+yROfE0A@public.gmane.org> > > _______________________________________________ > > Rails mailing list > > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > > http://lists.rubyonrails.org/mailman/listinfo/rails > > _______________________________________________ > Rails mailing list > Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org > http://lists.rubyonrails.org/mailman/listinfo/rails >_______________________________________________ Rails mailing list Rails-1W37MKcQCpIf0INCOvqR/iCwEArCW2h5@public.gmane.org http://lists.rubyonrails.org/mailman/listinfo/rails
Pat Maddox 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
On Wed, 2005-12-14 at 05:27 -0500, zdennis wrote:> Would you prefer? > > before_filter :authorize, :only=>[ :status, :admin ] > > Or.. > > def status > if authorize > ... > else > ... > end > end > > def admin > if authorize > ... > else > .,. > end > endFor 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 - Jamie
Jamie Macey wrote:> On Wed, 2005-12-14 at 05:27 -0500, zdennis wrote: > >>Would you prefer? >> >>before_filter :authorize, :only=>[ :status, :admin ] >> >>Or.. >> >>def status >> if authorize >> ... >> else >> ... >> 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 >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 12/15/05, zdennis <zdennis-aRAREQmnvsAAvxtiuMwx3w@public.gmane.org> wrote:> Jamie Macey wrote: > > On Wed, 2005-12-14 at 05:27 -0500, zdennis wrote: > > > >>Would you prefer? > >> > >>before_filter :authorize, :only=>[ :status, :admin ] > >> > >>Or.. > >> > >>def status > >> if authorize > >> ... > >> else > >> ... > >> 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 > > > > 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).
Bruce Balmer
2006-Jan-18 17:24 UTC
[Rails] What does ''OP'' mean in the context of this list?
> 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.
Alex Young
2006-Jan-18 17:27 UTC
[Rails] What does ''OP'' mean in the context of this list?
Original Poster (I presume) -- Alex Bruce Balmer wrote:>> 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. > > _______________________________________________ > Rails mailing list > Rails@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails
Bruce Balmer
2006-Jan-18 18:12 UTC
[Rails] What does ''OP'' mean in the context of this list?
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 On 18-Jan-06, at 10:30 AM, Alex Young wrote:> Original Poster (I presume) > > -- > Alex > > Bruce Balmer wrote: >>> 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. >> _______________________________________________ >> Rails mailing list >> Rails@lists.rubyonrails.org >> http://lists.rubyonrails.org/mailman/listinfo/rails > _______________________________________________ > Rails mailing list > Rails@lists.rubyonrails.org > http://lists.rubyonrails.org/mailman/listinfo/rails