[Rails] I am breaking the DRY principle - could I avoid it?

[Joe Van Dyk]

On 12/15/05, zdennis <zdennis at mktec.com> 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).