[rspec-users] Do you think it would look cleaner?

David Chelimsky dchelimsky at gmail.com
Sun Dec 30 07:12:26 EST 2007


On Dec 30, 2007 10:04 AM, David Chelimsky <dchelimsky at gmail.com> wrote:
> On Dec 30, 2007 6:52 AM, Jean-François Trân <jftran at rubyfrance.org> wrote:
> > 2007/12/29, Andrew WC Brown <omen.king at gmail.com>:
> >
> > > I just see these large blocks of:
> > >
> > > @shopping_list.should_receive(:milk)..and_return('milk')
> > >  @shopping_list.should_receive(:bagel).and_return('bagel')
> > > @shopping_list.should_receive(:coffee).and_return('coffee')
> > >
> > > and it would be much clearer if I could list them in a hash instead.
> >
> > What about :
> >
> > {
> >   :milk => 'milk',
> >   :bagel => 'bagel',
> >   :coffee => 'coffee'
> > }.each do |method, value|
> >   @shopping_list.should_receive(method).and_return(value)
> > end
> >
> > So it doesn't need a #should_receive_the_following method.
>
> You can certainly do that (and I have), but again, you'd lose the
> benefit of a unique line number in the failure message.
>
> This is something that is often lost under the banner of DRY. DRY
> doesn't mean "type as few characters as you can." It means don't
> repeat functionality or knowledge within a system. IMO, multiple calls
> to the same methods, but with different data, are NOT duplication in
> this sense.

(I hit send too soon)

In the end, DRY exists to serve a higher master: maintainability. And
it is not terribly rare that the illusion of DRY (as explored in this
case) flies in the face of maintainability. In this case, the clarity
of the line number in the event of a failure wins for me.

Also, this is going to have to change when you decide that the
shopping list should raise an error when it receives "bagel." Change
is fine, but to make that change is going to be more work and likely
leave things lopsided:

{
 :milk => 'milk',
 :coffee => 'coffee'
}.each do |method, value|
 @shopping_list.should_receive(method).and_return(value)
end

@shopping_list.should_receive(:bagel).and_raise(WeAreOutOfBagelsError)

To me, that is simply not as clear as this:

@shopping_list.should_receive(:milk).and_return('milk')
@shopping_list.should_receive(:coffee).and_return('coffee')
@shopping_list.should_receive(:bagel).and_raise(WeAreOutOfBagelsError)

FWIW,
David


More information about the rspec-users mailing list