Rails isn't DRY?

Rails
1

Hi all

I’m just exploring a bit the source code of Rails, and I looked at the
methods

validates_exclusion_of
validates_inclusion_of

The source code of both them is up to 99% exactly the same:

 # File vendor/rails/activerecord/lib/active_record/validations.rb,

line 602
602: def validates_exclusion_of(*attr_names)
603: configuration = { :message =>
ActiveRecord::Errors.default_error_messages[:exclusion], :on => :save }
604: configuration.update(attr_names.pop) if
attr_names.last.is_a?(Hash)
605:
606: enum = configuration[:in] || configuration[:within]
607:
608: raise(ArgumentError, “An object with the method include? is
required must be supplied as the :in option of the configuration hash”)
unless enum.respond_to?(“include?”)
609:
610: validates_each(attr_names, configuration) do |record,
attr_name, value|
611: record.errors.add(attr_name, configuration[:message]) if
enum.include?(value)
612: end
613: end

 # File vendor/rails/activerecord/lib/active_record/validations.rb,

line 575
575: def validates_inclusion_of(*attr_names)
576: configuration = { :message =>
ActiveRecord::Errors.default_error_messages[:inclusion], :on => :save }
577: configuration.update(attr_names.pop) if
attr_names.last.is_a?(Hash)
578:
579: enum = configuration[:in] || configuration[:within]
580:
581: raise(ArgumentError, “An object with the method include? is
required must be supplied as the :in option of the configuration hash”)
unless enum.respond_to?(“include?”)
582:
583: validates_each(attr_names, configuration) do |record,
attr_name, value|
584: record.errors.add(attr_name, configuration[:message])
unless enum.include?(value)
585: end
586: end

The only difference is on line 584/611. One neeeds if and the other one
needs unless.
Isn’t this a massive injure of the DRY principle? Couldn’t this have
been coded much cleaner by using the same method for both and only
switch if to unless when using the one or the other validation?

Thanks for your opinions,
Joshua

2

Probably. Maybe you could submit a patch?

Vish

3

On 30 Sep 2006, at 10:51, Joshua M. wrote:

The only difference is on line 584/611. One neeeds if and the other
one
needs unless.

576/603 are different as well. :wink:

Isn’t this a massive injure of the DRY principle?

sigh.

DRY is a great principle. We should all follow it. However it is a
principle, not a law. As a principle it exists so as to reduce the
complexity of the application with particular regard to maintenance.

However, sometimes you can push it to the point where maintaining the
code would be far more complicated than if you just did a cut and
paste. Sometimes it’s just easier for all concerned.

Let me give you an analogy to explain the principle better. Think
about the principles of normalisation within data modelling. In
theory, you should normalise all data to the maximum extent. That
would mean instead of having this schema:

Person:
Name:
Address_id:
Notes:

Address:
id:
Line1:
Line2:
City:
Postcode:
Country:

I should, according to the theorists who see normalisation as a law
to eliminate ALL duplication of data in the database, and not a
principle to help simplify the model, re-structure it as follows:

Person:
Name_id:
Address_id:
Note_id:

Names: # Two people might have the same name! Can’t duplicate data!
id:
Name:

Notes: # What if two people have the same notes?! That would be
duplication!
id:
Note:

Address:
id:
Line1_id: # Two people might have the same first line!
Line2_id # and second line! CAN’T DUPLICATE!
City_id: # Surely, more than one person lives in a city?!
Postcode_id: # Two people in the same area! Agghh! Duplication!
Country_id: # and the same country!

You get the idea. If you take normalisation to its maximum point to
completely and totally eliminate duplication, the result is a model
that is harder to code against, harder to maintain, a right PITA. The
simple truth is, DRY has to be sometimes seen in the same light. At
what point are you just normalising to the point of silliness?

In this particular example, we’re not talking about the same method
doing roughly the same thing - we’re actually talking about two
different methods, each just 10 lines long, with currently similar
internal behaviour but that may need to be extended in the future to
do rather different things regarding their behaviour.

Couldn’t this have been coded much cleaner by using the same method
for both and only switch if to unless when using the one or the
other validation?

I’m sure they’d be happy to consider a patch if you think it
worthwhile, but to say because of this one overlap that ‘Rails isn’t
DRY’ is a bit extreme in my opinion.

DRY is a principle. It is not a God to be worshipped, a law to be
observed on pain of ridicule, a boss to which you are a condemned
slave. It’s just a good idea, that’s all.

Thanks,

–
Paul R.
http://vagueware.com

4

I thought that part of the point of rails was to DRY up the
application development process. Correct me if I’m wrong, but nobody
involved in rails ever claimed that the code for the framework itself
was DRY, but applications developed using rails certainly are a lot
DRYer than many others.
-Nathan

5

Thanks for this interesting explanation. :slight_smile: