On Mon, Mar 2, 2009 at 8:35 PM, Stephen Eley <sfe...@gmail.com> wrote:
> On Mon, Mar 2, 2009 at 5:16 PM, Zach Dennis <zach.den...@gmail.com> wrote:
>>
>> Forgot to mention what we did do. We ended up with the following...
>>
>> def index
>> if user.has_role?("admin")
>> user.in_role("admin").invoices
>> elsif user.has_role?("associate")
>> user.in_role("associate").invoices
>> else
>> raise AccessDenied
>> end
>> end
>
> That seems sort of backwards to me. These aren't the user's invoices,
> right? They're just invoices which the user happens to be allowed to
> see? Chaining it this way makes it look like the invoices *belong* to
> the role, and seems put the user up front instead of the invoices.
> You also have conditional branching with hardcoded values, making the
> controller brittle, and some redundancy with the controller asking the
> model for a value and then passing the value right back to the model.
Agreed. I have a similar example in a blog post:
http://www.patmaddox.com/blog/2008/7/23/when-duplication-in-tests-informs-design
Pat
_______________________________________________
rspec-users mailing list
rspec-users@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
