Forum: RSpec Depot app, Demeter's law and troubles cleanly specing

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-18 19:17
Hi,

Let's take the example of the depot app. In my controller I set @order.
Then in the view comes the bad stuff:

@order.items.each do |item|
  item.product.title
end

Now I'm having problems specing item.product.title. A quick and dirty
fix is to trade a for for an underscore, so I create Item#product_title:

def product_title
  product.title
end

The problem is that I have a few models that act the same way, so I
would find myself writing lot's of these little instance methods, and
the solution looks a bit stupid to me. So how do you handle such issue?

I thought about composed_of, but AWDWR's examples still break Demeter's
law and that annoys me.


Thanks in advance
48641c4be1fbe167929fb16c9fd94990?d=identicon&s=25 Mark Wilden (Guest)
on 2009-04-18 20:54
(Received via mailing list)
On Sat, Apr 18, 2009 at 10:17 AM, Fernando Perez <lists@ruby-forum.com>
wrote:
> end
And this still doesn't satifsy the "Law" of Demeter because you're
still calling order.items[x].product_title. Simply adding a method
call in between doesn't obviate the fact that you need the order's
item's product's title.

I would split the difference and have an orders helper that knew how
to format each component of a displayed item.

///ark
54568ee7ba0c78a836e84c8756a3d681?d=identicon&s=25 Lenny Marks (Guest)
on 2009-04-19 00:09
(Received via mailing list)
On Apr 18, 2009, at 1:17 PM, Fernando Perez wrote:

> Now I'm having problems specing item.product.title. A quick and dirty
> issue?
>

Personally, I don't think its so bad to violate the 'Law of
Demeter'(which of course is really just a guideline) in cases such as
these dealing with domain models. All but the simplest apps will have
entities that relate to other entities and each entity will of course
have its own attributes. Sometimes I'll bring up a property from a
child entity if its extremely common(as may be the case with
product_title) but that's not the case most of the time. In this case,
IF I was trying to spec a view for an 'Order', I'd probably use a
factory method of some sort that would handle creating the desired
object graph. If these models already exist, I usually use real
objects in place of mocks. There are quite a few Factory/Builder type
libs out there for ActiveRecord. In my case, we're typically dealing
with Java/Hibernate domain layers through JRuby so I haven't used any
of them, but its been simple enough to roll my own so far.

it "should show product title" do
     order = new_order(:title => 'the product title')

     ....
end

-lenny
Cdf378de2284d8acf137122e541caa28?d=identicon&s=25 Matt Wynne (mattwynne)
on 2009-04-19 10:44
(Received via mailing list)
On 18 Apr 2009, at 23:08, Lenny Marks wrote:

>> item.product.title
>> The problem is that I have a few models that act the same way, so I
>> would find myself writing lot's of these little instance methods, and
>> the solution looks a bit stupid to me. So how do you handle such
>> issue?

The risk you've identified here is that your views are coupled to the
relationship between an order and the title of the products that make
up that order. If you were coding down in your model layer and decided
to make a change to the structure of your models then code far, far,
away in the views would break. In a small application like a blog,
this might not be a problem, but as your codebase starts to get
bigger, it's a worry you want to avoid.

Putting on your 'outside-in' hat and thinking about *what the view
wants*, you could code the view, which doesn't need or want to care
about the structure of these models, more like this:

   @order.product_titles do |product_title|
     <%= product_title %>
   end

Now you're going to need to add this Order#product_titles method to
your model layer. Having this extra method buys you more
maintainability in the future: the structure of the model objects is
hidden from the views, and if you need to change that structure, the
code you'll need to change to keep the views working
(Order#product_titles) is right down there next to you in the model
layer where you're working. This sort of cohesion - keeping the things
that change together in the same part of the codebase, makes
maintenance much easier and less accident-prone.

By designing a custom protocol (think 'API') on your model layer for
the use by the views, rather than constraining yourself to the ones
that come out of the box with ActiveRecord, you're putting some
lubrication in between the layers, making it easier for them to move
independently.

The trade-off, obviously, is that you're adding extra methods to your
models. If you imagine doing this for every view on a large project,
you might end up with quite a bit of clutter on those models. This is
where you could consider adding another 'facade' layer that wraps the
models and presents them to the views in the appropriate manner. At
Songkick, we use a presenter layer for this purpose, and it works very
nicely.

Matt Wynne
http://blog.mattwynne.net
http://www.songkick.com
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-19 12:08
>    @order.product_titles do |product_title|
>      <%= product_title %>
>    end
>

Another problem, is that not only do I need the title, but also a
clickable permalink which uses a url helper (not available to models),
the product price, and maybe other stuff in the future. So I might end
up with a nice messy code.

For the time being I use delegate, so I traded a dot for an underscore.
It's easy to spec, and after all, an item only exists in the context of
an Order, so I think it's fine to break Demeter's 'guideline' and say
that it's cool that Order knows a little bit stuff about Item's
internal. However a Product can exist without an Order, so Order doesn't
need to know anything about Product and vice versa.

But I'll have to think more about it, because as you said making a
little change in a model can result in having to fix places that are far
far away and totoally unexpected.
Cdf378de2284d8acf137122e541caa28?d=identicon&s=25 Matt Wynne (mattwynne)
on 2009-04-19 12:55
(Received via mailing list)
On 19 Apr 2009, at 11:08, Fernando Perez wrote:

>>   @order.product_titles do |product_title|
>>     <%= product_title %>
>>   end
>>
>
> Another problem, is that not only do I need the title, but also a
> clickable permalink which uses a url helper (not available to models),
> the product price, and maybe other stuff in the future. So I might end
> up with a nice messy code.

This is where I start to introduce a presenter layer in between the
view and the models.

If you have a Ruby class anywhere in rails, you can access the auto-
generated url helpers just by including ActionController::UrlWriter.

class OrderPresenter
   include ActionController::UrlWriter

   def initialize(order)
     @order = order
   end

   def product_titles
     @order.items.map{ |i| i.product.title, product_url(i.product) }
   end
end

Construct the presenter in the Controller. Then in the view:

@order_presenter.product_titles do |product_title, url|
   <%= link_to product_title, url %>
end

> But I'll have to think more about it, because as you said making a
> little change in a model can result in having to fix places that are
> far
> far away and totoally unexpected.
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users

Matt Wynne
http://beta.songkick.com
http://blog.mattwynne.net
48641c4be1fbe167929fb16c9fd94990?d=identicon&s=25 Mark Wilden (Guest)
on 2009-04-19 18:40
(Received via mailing list)
On Sun, Apr 19, 2009 at 3:51 AM, Matt Wynne <matt@mattwynne.net> wrote:

> @order_presenter.product_titles do |product_title, url|
>  <%= link_to product_title, url %>
> end

The presentation of an order in most apps will be constructed from
additional columns (e.g., color, size, price, extended price). These
columns may well be formatted differently depending on the
circumstance (e.g, printed or on-screen).

Now, it might be a good idea to put this construction and formatting
in a helper or presenter outside the view (though it won't be
TSTTCPW). But anywhere you put it, there will be non-Demeterian code
that needs to drill down into each order's items', products'
information (as well as non-product information, such as line-item
discount). If the underlying model changes, this code will have to
change - you can't avoid it.

///ark
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-19 19:10
> This is where I start to introduce a presenter layer in between the
> view and the models.

Very interesting. Thank you very much.
F86901feca747abbb5c6c020362ef2e7?d=identicon&s=25 Zach Dennis (zdennis)
on 2009-04-19 20:29
(Received via mailing list)
On Sun, Apr 19, 2009 at 12:24 PM, Mark Wilden <mark@mwilden.com> wrote:
>
> Now, it might be a good idea to put this construction and formatting
> in a helper or presenter outside the view (though it won't be
> TSTTCPW). But anywhere you put it, there will be non-Demeterian code
> that needs to drill down into each order's items', products'
> information (as well as non-product information, such as line-item
> discount). If the underlying model changes, this code will have to
> change - you can't avoid it.

Agreed. I think Dan Manges post is a good one on this topic as well:

     http://www.dcmanges.com/blog/37

His second to last paragraph:

"The crux of this is that webpage views aren't domain objects and
can't adhere to the Law of Demeter. Clearly from the examples of
behavior delegation the Law of Demeter leads to cleaner code. However,
when rendering a view, it's natural and expected that the view needs
to branch out into the domain model. Also, anytime something in a view
dictates code in models, take caution. Models should define business
logic and be able to stand alone from views. If this "train-wreck"
method calling in your views is bothersome, there is a better solution
that I will blog about later."

I tend to agree his thoughts on this, but be careful how you read the
sentence "anytime something in a view dictates code in models, take
caution". He isn't referring outside-in development practices. He's
referring to  adding methods in a model for strictly presentation
purposes, which is bad because it dilutes the richness of the model,
and makes it change for reasons it shouldn't.  Here's another article
which discusses why methods shouldn't be added to models for
presentation purposes:

   http://spin.atomicobject.com/2008/01/27/the-except...

Although if you read that article and want to try out presenters, you
should know that CachingPresenter supersedes the PresentationObject
library in the article. And you don't need a library to use
presenters, your presenters can be POROs just like Matt Wynne posted.
I tend to like the CachingPresenter library (heck I wrote it) approach
so I can utilize caching, memoization, and a slightly more declarative
API and list of helper methods.

One technique I've employed in the past along side presenters
(although it can be done w/o presenters) is to make use of partials,
and actually treat them like little view objects. So you could take
what you had:

   @order.items.each do |item|
     item.product.title
   end

And transform it into two views, one that is concerned with the order,
and a partial which is concerned with the product of an item. :

   <%= render :partial "items/product", :collection =>
@order.items.map(&:product) %>

And then add a "items/product" partial and spec it in isolation.
Spec'ing views (and partials) in isolation gives you the ability to
have simpler view specs, and you're able to avoid feeling the awkward
need to build up a network of objects. And you don't have to worry
about adding a bunch of little delegate methods all over the place.


--
Zach Dennis
http://www.continuousthinking.com
http://www.mutuallyhuman.com
5d38ab152e1e3e219512a9859fcd93af?d=identicon&s=25 David Chelimsky (Guest)
on 2009-04-19 21:06
(Received via mailing list)
On Sun, Apr 19, 2009 at 11:24 AM, Mark Wilden <mark@mwilden.com> wrote:
>
> Now, it might be a good idea to put this construction and formatting
> in a helper or presenter outside the view (though it won't be
> TSTTCPW). But anywhere you put it, there will be non-Demeterian code
> that needs to drill down into each order's items', products'
> information (as well as non-product information, such as line-item
> discount). If the underlying model changes, this code will have to
> change - you can't avoid it.

The motivation behind demeter is to localize change. The cost is
method bloat and potential lack of cohesion on a model. Having a
single presenter object act as the one and only place that will change
besides the model itself is a good compromise between complete
localization of change in the model and method bloat on the model.

A very interesting approach to all this was presented by Allen Holub
in his talk boldly entitled "Everything You Know is Wrong," in which
he tears apart common misunderstandings about OO.

http://www.holub.com/publications/notes_and_slides...

The idea, as I understand it (but, according to his basic premise, I'm
probably wrong) is to have data importers and exporters on domain
objects in order to minimize getters and setters. These collaborators
are going to take the hit of changes to the model objects, but *only*
they will if you follow this approach. In the example we're talking
about here, we'd end up with code like this in the controller:

def some_view
  @order = find_order.export_to(OrderPresenter.new)
end

OrderPresenter would have a bunch of setters on it, which *the order,
which knows its own data structure* would call. Now you don't need a
bunch of getter methods on order.

Similarly, data importers would have a bunch of getters on them, and
you would use them to import data into an order like this:

order_form = OrderForm.new(params[:order])
order = Order.create_from(order_form)

Obviously, ActiveRecord provides the getters and setters anyway, but
the real violator of encapsulation is the consumer, not the vendor.
Just because a flasher opens his coat doesn't mean that reaching
inside is a good idea ;)

Cheers,
David
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-19 21:37
>    http://spin.atomicobject.com/2008/01/27/the-except...
Interesting idea too. So basically I need to totally rethink and
refactor the way my views display the information to the customer:

Let's see how I currently display or not an add to cart button depending
whether or not the product is free.

My very first quick and very ugly procedural hard to spec solution was
to do in the view:

<%- if @product.price >= 0 -%>
  <%= display_button %>
<%- else -%>
  This product is free!
<%- end -%>

Then my second solution was to create an instance method so that the
view doesn't know about the Product internal mechanism about it's
freeness:

<%- if @product.free? -%>
 This product is free!
<%- else ... -%>

Yeah I thought I was an OOP master and Demeter could rest in peace!

But thinking about the GOF Builder and Exceptional Presenter design
patterns you talk about, that would mean that the html output for a
Product should therefore happen in the Model itself. And then in the
show.html.erb, I simply call:
<%= @product.display -%>

and all the magic about whether or not the product is free and to
display the button has already been handled inside the model (or another
related place) when it gets instantiated. Same applies to
item.product.title, that would be handled elsewhere than in the view.

However it might clutter the Model, so actually there is more to MVC:
each Model should have a sub class or something that handles how the
model instance will be presented to the view.

How do you handle such issue? Are there some open source rails apps that
I could learn from?
Cdf378de2284d8acf137122e541caa28?d=identicon&s=25 Matt Wynne (mattwynne)
on 2009-04-19 22:44
(Received via mailing list)
On 19 Apr 2009, at 19:54, David Chelimsky wrote:

>> additional columns (e.g., color, size, price, extended price). These
>
> http://www.holub.com/publications/notes_and_slides...
> end
>
> OrderPresenter would have a bunch of setters on it, which *the order,
> which knows its own data structure* would call. Now you don't need a
> bunch of getter methods on order.
>
> Similarly, data importers would have a bunch of getters on them, and
> you would use them to import data into an order like this:
>
> order_form = OrderForm.new(params[:order])
> order = Order.create_from(order_form)

I think (but I'm probably wrong too!) that this is what people like
Nat Pryce and Steve Freeman are describing when they talk about
keeping state and behaviour separate. It's something that functional
programming languages like Haskell force you to do, but it's decent
practice to use day-to-day even in languages that let you mix the two
together, IMO.

> Obviously, ActiveRecord provides the getters and setters anyway, but
> the real violator of encapsulation is the consumer, not the vendor.
> Just because a flasher opens his coat doesn't mean that reaching
> inside is a good idea ;)

LOL. I had half-written my own reply to Mark, but that says it all :)


Matt Wynne
http://blog.mattwynne.net
http://www.songkick.com
B8c4bfb1b6002a5b43d87de3335401f5?d=identicon&s=25 BJ Clark (Guest)
on 2009-04-19 23:09
(Received via mailing list)
The way I go about this is, IMO, pretty straight forward. I cringe
when people start talking about generating html in models, since I
think separation of concerns is much more important than law of
demeter, and I think that the model most definitely shouldn't be
concerned with how it needs to be displayed.

In your original example you have:
@order.items.each do |item|
  item.product.title
end

The way I would do this is:
@order.items.each do |item|
render :partial => "product/line_item", :locals => {:product =>
item.product}
end

In other words, I might have the following partials:
product/_line_item
product/_product (full representation of product probably for detail
page)
product/_search_item (for display in a search result)

etc.

All the partials simply take a product object. There is probably
already some smart, intention revealing name, but I call them
"nanoformats".
I'm not exactly an Academic, so I have no idea the validity of my
approach, but this seems to get around the law of demeter enough for
me (thought, obviously not totally since order still *technically*
knows about order.items.products). Order doesn't actually know
anything about products or it's internals, except that it has some.
The partial could be reused in other places, and with semantic markup,
can be displayed in very different ways with a little CSS. A change to
Product only necessitates changes to the app/views/products/* files.
Your Orders don't need to know anything, item probably doesn't even
need to be updated. the partials can be spec'd independently of
wherever they are being used.

Your view and your models are always going to be tightly coupled, and
a change in your domain object should probably necessitate a change in
your views. That's the way it *should* work. However, your views
shouldn't be tightly coupled with each other but they will have to be
coupled in some way or another.

-----
BJ Clark
85d99e7678d8720f6e00ab0f60fe6ea9?d=identicon&s=25 Andrew Premdas (Guest)
on 2009-04-20 01:50
(Received via mailing list)
+1 what we are doing here is rendering resources in different contexts.
In
the original example we are rendering product resources in an order
context.
We should let the product define how it should be viewed - after all it
knows best. If we need different views in different contexts then just
create a new partial for product.

Generally for any resource I will create partials for following views
  - detail
  - summary
  - admin_summary

And create other partials for forms etc.

Another plus about this approach is that it helps guide consistency in
view
e.g. products look the same in 'my recent orders', 'search for product
foo',
'my favourite products' ...


2009/4/19 BJ Clark <bj@aboutus.org>
39100495c9937c39b2e0c704444e1b4a?d=identicon&s=25 Pat Maddox (Guest)
on 2009-04-20 21:44
(Received via mailing list)
I wrote a blog post that may be helpful.
http://www.patmaddox.com/blog/demeter-is-for-encapsulation  Basically,
when you have structural objects as in this case, demeter isn't
useful.

Luke Redpath wrote something called Demeter's Revenge which you might
want to look at.  I've not used it though so I can't tell you if it
will solve your problems.

Pat
61c19adc48dad558ed3b55c5dc0b4f10?d=identicon&s=25 Steve Schafer (Guest)
on 2009-04-21 15:57
(Received via mailing list)
On Mon, 20 Apr 2009 12:40:08 -0700, you wrote:

>I wrote a blog post that may be helpful.
>http://www.patmaddox.com/blog/demeter-is-for-encapsulation  Basically,
>when you have structural objects as in this case, demeter isn't
>useful.

That's a good example as far as it goes, but I think it makes a
distinction that isn't quite the true distinction. For example, I
remember dragsters from the '60s that had as many as four engines. What
does car.engine.name do in that case? And the car I drive these days has
a pair of electric motors in addition to an internal combustion
engine...

Likewise with the stereo--some minivans and SUVs come with two
independent stereo/entertainment systems, one for the front seat
passengers and one for the rear. When I invoke car.stereo.has_cd_player,
am I asking about the "default" front-seat stereo, or do I only care
whether or not the car has a cd player somewhere?

The point, of course, is that digging down into the structure of an
object, even if structural information is what you're after, requires
that you make assumptions about that structure. And _that_ should be the
criterion. You have to ask yourself:

(a) What implicit assumptions am I making by doing this?

(b) Is is safe to make those assumptions?

If you can reasonably answer yes to the second question, then you're in
good shape. But you still have to accept that future changes to the
design can lead to structural changes that retroactively invalidate your
answer.

-Steve
54568ee7ba0c78a836e84c8756a3d681?d=identicon&s=25 Lenny Marks (Guest)
on 2009-04-22 19:37
(Received via mailing list)
On Apr 21, 2009, at 9:45 AM, Steve Schafer wrote:

> On Mon, 20 Apr 2009 12:40:08 -0700, you wrote:
>
>> I wrote a blog post that may be helpful.
>> http://www.patmaddox.com/blog/demeter-is-for-encapsulation
>> Basically,
>> when you have structural objects as in this case, demeter isn't
>> useful.

I agree 100%.

>
> the
> your
> answer.
>

I'm not sure how this ties to the application of Demeter's law. I
think Pat's point was that if what you're interested in is an object's
structure(such as to display information about the engine/engines in a
web page because customers want to see that) then applying Demeter's
law is just "needless indirection without any benefits". The decision
to model a car with Car#stereo Car#engine vs. Car#engines Car#stereos
has to be made(risk and assumptions considered here).  If that
changes, things that renderinformation about the car engine
information WILL have to change. At that point I don't see how
Car#engine_name, Car#engine_type is of any benefit over
Car#engine.name. Car#engine_type. This is in contrast to the same
references in the context of behavior(e.g. car.start() vs.
car.engine.start()).  I'm REALLY missing how applying GoF Builder
solution from *Everything.You.Know.is.Wrong to avoid getters/setters
buys me anything at all. I guess it would prevent the possibility of
behavior type code that violates Demeter's law, but it seems like it
comes at high cost. Unnecessary redundancy and class
explosion(importer/exporter interface for every domain class).  This
seems more doable in a dynamic language with method_missing type
stuff, but I can't even imagine this in Java. Now every time I need to
add a new attribute to a class, I have to change all the Importer/
Exporter implementations regardless of their interest in said
attribute? How does that make my code simpler and more maintainable?

-lenny

*
http://www.holub.com/publications/notes_and_slides...
059ed46172a087063ce26250e44c8627?d=identicon&s=25 Fernando Perez (fernando)
on 2009-04-23 20:19
Rails definitely entices you to break Demeter's law just so often.

Now how to cleanly spec:

@comment = @article.comments.build(params[:comment]) ?

Mocking and stubbing is starting to get ugly now.
F86901feca747abbb5c6c020362ef2e7?d=identicon&s=25 Zach Dennis (zdennis)
on 2009-04-23 20:26
(Received via mailing list)
On Thu, Apr 23, 2009 at 2:19 PM, Fernando Perez <lists@ruby-forum.com>
wrote:
> Rails definitely entices you to break Demeter's law just so often.
>
> Now how to cleanly spec:
>
> @comment = @article.comments.build(params[:comment]) ?

You think that's bad, I've seen many a code that looks like:

project.tasks.map(&:responsible_party).group_by{ ... }.map....sort

>
> Mocking and stubbing is starting to get ugly now.
>
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> rspec-users mailing list
> rspec-users@rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>



--
Zach Dennis
http://www.continuousthinking.com (personal)
http://www.mutuallyhuman.com (hire me)
@zachdennis (twitter)
994e42bda994be2cd1d791f18ee6d561?d=identicon&s=25 Stephen Eley (Guest)
on 2009-04-23 21:41
(Received via mailing list)
On Thu, Apr 23, 2009 at 2:19 PM, Fernando Perez <lists@ruby-forum.com>
wrote:
> Rails definitely entices you to break Demeter's law just so often.

So fix it.  It's usually just a matter of putting in some delegators.
If you don't like @article.comments.build, you can declare your own
Article.build_comment() method and simply have it point to the
comment's method.  Or call it something else if you don't like the
word "build."  (I don't.)


> Now how to cleanly spec:
> @comment = @article.comments.build(params[:comment]) ?

That's backwards.  If you already have a line of code and you're
asking how to write a spec for it, you're not honoring the behavior.
You're honoring that line of code.

I'm going to guess from knowledge of the idiom that this is for a
'create' method in the CommentsController, nested under
ArticlesController.  Whether it calls 'comments.build' or
'comments.create' or 'make_me_a_comment!' isn't important and doesn't
need to be specified.  What's important is the end result of the
method, which is that the article gains a shiny new comment:

# Assume an article with no comments and a params structure
# have already been set up in before(:each)

it "should make a new comment belonging to the article" do
  post :create, :comment => @my_valid_params
  @my_article.should have(1).comment
end

How clean is that?  Now just a couple more specs for *invalid* params
and for where the method directs to, and you're basically done.


> Mocking and stubbing is starting to get ugly now.

That much is true.  You'll notice I didn't mock or stub anything up
there.  I honestly think the controller code by the RSpec scaffold
generator is the wrong approach; it's specing implementation, not
behavior.  Even worse, it's specing the implementation details of
*model* code, which shouldn't even be a consideration here.  My way
breaks isolation (it hits the database) but it's a lot simpler and
focuses on behavior.

Is it possible to get behavior focus *and* isolation?  I started to
think about that.  And then I realized, in accordance with the "If
it's hard to spec you're probably doing it wrong" principle,
that...well, Rails is probably doing it wrong.  Controllers in Rails
are just doing the Wrong Thing.  That's why specing them is so
painful, and why so many of us skip trying.

What's the right way?  I'm still pondering.

Yeah, I'm a smartass.


--
Have Fun,
   Steve Eley (sfeley@gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org
Fd8f09626613a93a79e2ae899f00a465?d=identicon&s=25 Dan North (Guest)
on 2009-04-27 16:04
(Received via mailing list)
2009/4/24 Stephen Eley <sfeley@gmail.com>

>  @my_article.should have(1).comment
> end
>
> How clean is that?  Now just a couple more specs for *invalid* params
> and for where the method directs to, and you're basically done.


Amen to that.


> > Mocking and stubbing is starting to get ugly now.
>
> That much is true.  You'll notice I didn't mock or stub anything up
> there.  I honestly think the controller code by the RSpec scaffold
> generator is the wrong approach; it's specing implementation, not
> behavior.  Even worse, it's specing the implementation details of
> *model* code, which shouldn't even be a consideration here.  My way
> breaks isolation (it hits the database) but it's a lot simpler and
> focuses on behavior.


And amen to that too.

Even within an rspec example I think of each line of code as a Given
(setup), When (event) or Then (outcome). Usually when I see a line doing
more than one of these - say calling a method and verifying its result -
I
break it out into separate event and outcome lines.

The reason for this is that the givens and outcomes can tinker with
whatever
they want. They can create mocks, poke around in the database, wire up
models, whatever. The event lines though - the Whens - can only interact
with the object like client code would. In other words they can't know
that
an object is a mock, or that a value got into a database because you
poked
it there.

Your example is great. You have an event line that interacts with the
controller through a regular HTTP POST, like a client would, and you
verify
that by checking the database via the ActiveRecord model object. There
is no
need for a mock here if your example is checking that data ends up in
the
database (although that does make it more of an integration test than a
behavioural spec). Because of rails's evil insistence that a "model" is
just
a persistence strategy, they are pretty much the same thing.

Is it possible to get behavior focus *and* isolation?


Yes, but it only has value on a per-line-of-code-in-your-spec basis.
Your
givens and outcomes shouldn't care about isolation - your events should
only
be exercising behaviour that is available in the object you are
describing.

It gets more complicated when a single line of code both exercises
behaviour
and does verification. I find breaking these out helps my sanity.

 I started to
> think about that.  And then I realized, in accordance with the "If
> it's hard to spec you're probably doing it wrong" principle,
> that...well, Rails is probably doing it wrong.  Controllers in Rails
> are just doing the Wrong Thing.  That's why specing them is so
> painful, and why so many of us skip trying.
>
> What's the right way?  I'm still pondering.


Me too!


> Yeah, I'm a smartass.


Well you seem to be on the right lines to me.

Cheers,
Dan
5d38ab152e1e3e219512a9859fcd93af?d=identicon&s=25 David Chelimsky (Guest)
on 2009-04-27 16:31
(Received via mailing list)
On Sun, Apr 26, 2009 at 9:20 PM, Dan North <tastapod@gmail.com> wrote:
>> it "should make a new comment belonging to the article" do
>> > Mocking and stubbing is starting to get ugly now.
>
> it there.
>
>> that...well, Rails is probably doing it wrong.  Controllers in Rails
>> are just doing the Wrong Thing.  That's why specing them is so
>> painful, and why so many of us skip trying.

I've struggled with this bit myself, as I suspect many of us who did
TDD before Rails have.

RSpec's mocking/stubbing framework just got a contribution that allows
you to do this in a controller spec:

Thing.stub_chain(:all, :with_some_scope, :perhaps_another).
  and_return([stub_model(Thing)]

This let's you stub a chain of named scopes, for example, in a rails
controller. Similarly, with models:

member.stub_chain(:addresses, :first, :zip_code)

Admittedly, this can result in an excuse for poor design choices. In
this example, the first address probably has some meaning that could
be exposed through a method like primary_address() or some such. Even
in that case, I think that allowing
member.stub_chain(:primary_address, :zipcode) would be preferable over
adding methods like primary_address_zipcode(). Of course, a little
method_missing magic could solve that as well :) I guess it's good to
have options.
994e42bda994be2cd1d791f18ee6d561?d=identicon&s=25 Stephen Eley (Guest)
on 2009-04-28 03:02
(Received via mailing list)
On Sun, Apr 26, 2009 at 10:20 PM, Dan North <tastapod@gmail.com> wrote:
> 2009/4/24 Stephen Eley <sfeley@gmail.com>
>>
>> Yeah, I'm a smartass.
>
> Well you seem to be on the right lines to me.

Thank you!  That means a great deal to me.


--
Have Fun,
   Steve Eley (sfeley@gmail.com)
   ESCAPE POD - The Science Fiction Podcast Magazine
   http://www.escapepod.org
93e9084aa289b7f1f5e4ab6716a56c3b?d=identicon&s=25 Emerson Macedo (emerleite)
on 2009-11-27 04:24
Fernando Perez wrote:
> Hi,
>
> Let's take the example of the depot app. In my controller I set @order.
> Then in the view comes the bad stuff:
>
> @order.items.each do |item|
>   item.product.title
> end
>
> Now I'm having problems specing item.product.title. A quick and dirty
> fix is to trade a for for an underscore, so I create Item#product_title:
>
> def product_title
>   product.title
> end
>
> The problem is that I have a few models that act the same way, so I
> would find myself writing lot's of these little instance methods, and
> the solution looks a bit stupid to me. So how do you handle such issue?
>
> I thought about composed_of, but AWDWR's examples still break Demeter's
> law and that annoys me.
>
>
> Thanks in advance

Hi, please try the Law of Demeter gem

Its a DRY way to apply Law of Demeter with demeter gem

http://github.com/emerleite/demeter
http://gemcutter.org/gems/demeter
5d38ab152e1e3e219512a9859fcd93af?d=identicon&s=25 David Chelimsky (Guest)
on 2009-11-27 12:16
(Received via mailing list)
On Thu, Nov 26, 2009 at 9:24 PM, Emerson Macedo
<lists@ruby-forum.com>wrote:

> > Now I'm having problems specing item.product.title. A quick and dirty
> > I thought about composed_of, but AWDWR's examples still break Demeter's
> > law and that annoys me.
> >
> >
> > Thanks in advance
>
> Hi, please try the Law of Demeter gem
>
> Its a DRY way to apply Law of Demeter with demeter gem
>

Interesting. There is a plugin still floating around called Demeter's
Revenge that does a similar thing with associations rather than
attributes.
So, in the demeter gem, you can do this:

@post.author_name # same as @post.author.name

Whereas in demeters_revenge you can do this:

@post.create_author # same as @post.author.create

Be nice if both sets of behaviour were in one gem :)
This topic is locked and can not be replied to.