thr3ads.net - Rails - Rails isn''t DRY? [Sep 2006]

If this information is useful, please help other people find it:
Share via:

Joshua Muheim

2006-Sep-30 09:51 UTC

Rails isn''t DRY?

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

-- 
Posted via http://www.ruby-forum.com/.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk
-~----------~----~----~----~------~----~------~--~---

Vishnu Gopal

2006-Sep-30 13:23 UTC

head link

Re: Rails isn''t DRY?

Probably. Maybe you could submit a patch?

Vish

On 9/30/06, Joshua Muheim
<rails-mailing-list-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org>
wrote:>
>
> 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
>
> --
> Posted via http://www.ruby-forum.com/.
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk
-~----------~----~----~----~------~----~------~--~---

Paul Robinson

2006-Sep-30 13:45 UTC

head link

Re: Rails isn''t DRY?

On 30 Sep 2006, at 10:51, Joshua Muheim wrote:
> The only difference is on line 584/611. One neeeds if and the other  
> one
> needs unless.
576/603 are different as well. ;-)
> 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 Robinson
http://vagueware.com

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk
-~----------~----~----~----~------~----~------~--~---

Joshua Muheim

2006-Sep-30 13:54 UTC

head link

Re: Rails isn''t DRY?

Thanks for this interesting explanation. :-)

-- 
Posted via http://www.ruby-forum.com/.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk
-~----------~----~----~----~------~----~------~--~---

njmacinnes-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

2006-Sep-30 14:33 UTC

head link

Re: Rails isn''t DRY?

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

On 30/09/06, Joshua Muheim
<rails-mailing-list-ARtvInVfO7ksV2N9l4h3zg@public.gmane.org>
wrote:>
> Thanks for this interesting explanation. :-)
>
> --
> Posted via http://www.ruby-forum.com/.
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk-unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk
-~----------~----~----~----~------~----~------~--~---

Rails - Sep 2006 - Rails isn't DRY?

Rails isn''t DRY?

Re: Rails isn''t DRY?

Re: Rails isn''t DRY?

Re: Rails isn''t DRY?

Re: Rails isn''t DRY?