How DRY is too DRY?

Rails
1

I’m working on fixing up the code in my first Rails application and
have come around to DRYing up the views and partials. For the most
part, I’m satisfied with the DRYness of the application, but I have
5-6 partials that display nearly identical XHTML, just with different
content, link_to paths, and images.

I know this question is entirely subjective, but would finding a way
to further DRY the 5-6 partials into one partial with plugs for
different fillers be too much? If so, how would I go about doing
this?

2 examples…

http://pastie.org/402754
http://pastie.org/402756

2

Here’s how it could look like → http://pastie.org/402767

And having five pages with almost the same markup is always wrong, you
don’t even need to be looking for DRYing up your code.

Maurício Linhares
http://alinhavado.wordpress.com/ (pt-br) | http://blog.codevader.com/
(en)

3

And what about cases where I need the model attribute names to be
different? For instance, in one case I might print item.name, but in
the other, item.title. And since this markup will be similar to the
front-end display (non-admin), is there a good way to set the link_to
to be either :admin or :root?

On Feb 27, 9:35 pm, Maurício Linhares [email protected]

4

Thanks, I’ll work through your example.

I’m not sure that I follow your second comment. I understand that
having duplicate markup is not desirable, but is there another way I
should be approaching this?

On Feb 27, 9:35 pm, Maurício Linhares [email protected]

5

On Fri, Feb 27, 2009 at 11:50 PM, ericindc [email protected]
wrote:

And what about cases where I need the model attribute names to be
different? For instance, in one case I might print item.name, but in
the other, item.title.

Make all models that are going to be used by that template respond to
the same message. If at your template, you’re calling “item.title”,
all models that are going to be used in there should have a “title”
method. In your case, you can just alias the “name” method to “title”.
A better approach would be to implement the “to_s” method in your
active record models by returning these “name” properties.

If you have a user model, you could implement it’s to_s like the
following:

class User < ActiveRecord::Base

def to_s
login
end

end

So you don’t even care about what that model is, you just expect it’s
to_s method to yield a correct “label”.

And since this markup will be similar to the
front-end display (non-admin), is there a good way to set the link_to
to be either :admin or :root?

You can just add another parameter at the render :partial call.

Maurício Linhares
http://alinhavado.wordpress.com/ (pt-br) | http://blog.codevader.com/
(en)

6

ericindc wrote:

I’m working on fixing up the code in my first Rails application and
have come around to DRYing up the views and partials. For the most
part, I’m satisfied with the DRYness of the application, but I have
5-6 partials that display nearly identical XHTML, just with different
content, link_to paths, and images.

This supplements your other answers, which are doubtless technically
correct.

The DRY is primarily about the behaviors of methods in an OO design.
Having two
methods that do the exact same thing, but with different labels or a
different
algorithm, is the greatest violation of DRY. (Start by seeing if you can
make
the two methods look more similar, before merging them. If you can’t, at
least
find a way to put them right next to each other!)

Similarly, your metadata should be DRY. This is why (and how) Rails
obeys
“convention before configuration”. Don’t write an XML file saying “our
unit
tests are in the test/ folder”. That repeats the information stored in
the name
of the folder.

Now we come to the heart of the matter. HTML, by itself, is not
behavior, and
it’s almost not structure. It is mostly art. DRYing two similar
stretches of
HTML is the moral equivalent of DRYing all your #fff and “white” CSS
directives.
It’s okay if they say the same thing, when they don’t use logical
statements to
say them, and when you might not frequently need to change them all at
the same
time.

So that is the answer to “too DRY”. Keep DRYing your project until you
reach a
local maximum of convenience, and then don’t DRY any more.

–
Phlip

7

Ok, I follow having all models respond to “title”, but why would
implementing the model’s “to_s” method be a better approach? Is it
just for consistency, always expecting to_s to return what the partial
should display? How is that any different that expecting all models
to answer to title?

Thanks. This has been really helpful. I used your example to create
the following. Thoughts?

http://pastie.org/402843

On Feb 27, 11:07 pm, Maurício Linhares [email protected]

8

On Sat, Feb 28, 2009 at 1:32 AM, ericindc [email protected] wrote:

Ok, I follow having all models respond to “title”, but why would
implementing the model’s “to_s” method be a better approach? Is it
just for consistency, always expecting to_s to return what the partial
should display? How is that any different that expecting all models
to answer to title?

First it’s for the sake of consistency, but it’s also 'cos the to_s
method is defined to return a “string representation of the object”,
so if you already have a method that does what you want, why create
another one or why care about creating another method that “returns a
string representation of the object”?

This way you don’t need to add extra virtual methods at your models,
you just expect them to have a good to_s implementation.

Thanks. This has been really helpful. I used your example to create
the following. Thoughts?

http://pastie.org/402843

It’s good, but that’s how I’d do it → http://pastie.org/403009

The more conventions you have, the easier it is to keep things
consistent in your pages.

Maurício Linhares
http://alinhavado.wordpress.com/ (pt-br) | http://blog.codevader.com/
(en)

9

Thanks for both posts regarding DRY and too DRY. Some very good
points.

On Feb 28, 7:08 am, Maurício Linhares [email protected]
wrote:

so if you already have a method that does what you want, why create
another one or why care about creating another method that “returns a
string representation of the object”?

Thanks, you’ve been a huge help.

Makes sense for consistency’s sake. My initial concern is about
overriding a core method. What if I needed to_s elsewhere? Plus,
wouldn’t I be writing more code given that every model must now have a
custom defined to_s method? Unless adding extra virtual attributes is
bad, would it be less work (given that I’d have to change two models
at this point) to just add a “title” method for those that doesn’t
respond to title?

The more conventions you have, the easier it is to keep things
consistent in your pages.

I picked up a few good points, but I decided on my implementation
because there are other types of those sidebar boxes. For instance,
they won’t all say “Create New XYZ”. Some will say “My Comments”, or
“Add Member”. The images, etc. will change too.

It made sense to me to keep the structure of the box DRY since that
will never change and just plug in my values as needed. And the only
way I found to do that was what I pastied too. I’ll keep looking at
it though.

10

this?
My criteria for repeat detection is, what you want when you need
to change them. When you DRY up two similar parts, you lose the ability
to modify one without affect the other one. So I think if the similar
parts are likely to change in the same way in future, they should be
DRYed; otherwise I prefer to keep them seperate although they looks
simliar to each other.

my $0.02
Jan

11

On Sat, Feb 28, 2009 at 10:02 AM, ericindc [email protected]
wrote:

Makes sense for consistency’s sake. My initial concern is about
overriding a core method. What if I needed to_s elsewhere?

Usually, you shouldn’t need it unless what you need is a “string
representation of your object” (yes, i’m sounding like a broken vinyl
now ;D), and receiving a user_name or a title would probably be what
you’re looking for.

Also, if it’s something for debugging purposes in Ruby we use “inspect”.

Plus, wouldn’t I be writing more code given that every model must now have a
custom defined to_s method? Unless adding extra virtual attributes is
bad, would it be less work (given that I’d have to change two models
at this point) to just add a “title” method for those that doesn’t
respond to title?

Yes and no. When you say that it’s “title”, it has to be a method
called title, so you’re stuck with this “title” thing, if a model has
a title and you don’t want to use it, you’re about to have a problem.
Using to_s or a to_label (this one feels clumsy) means your view code
don’t need to know which method is going to be called, which
simplifies changes in the future. Imagine that today, when presenting
a user, you show it’s full name, but later on there’s a request to,
instead of showing the full name, show only the login, if you had a
full_name call at every user show, you’d have to change them all to
login, but if you had oveeriden the to_s method you would just have to
change it in one place.

Also, when you override the to_s method you don’t need to call it at
the view, as you need to call the “title” method:

<%= link_to user, user %>

The link_to helper will automatically call “to_s” at your object, so
you don’t have to call it by yourself.

Maurício Linhares
http://alinhavado.wordpress.com/ (pt-br) | http://blog.codevader.com/
(en)

12

Ok, I’m sold. I’ll update each model’s to_s method to respond with
the “title” that I want displayed in the partial. Thanks again for
your input.

On Feb 28, 8:31 am, Maurício Linhares [email protected]