DRY your scopes

I encountered a pattern today that I would like to share:

There is no justification to write such a code in ruby (or in any language)
ActiveRecord 3 with ARel does not defer class singleton methods from scopes and in my opinion they are more readable.
We could write the same functionality with:

Now, the path is short to go to:

I like it – do you?

We didn’t only clean the code.
We did much more – every new status in the STATUS list will effortlessly represented with a matching method.

For instance :curator key which had no scope in the original version is now represented with the User.curator method.

DRY is not (just) about aesthetics. DRY is about maintainability and simplicity.

Advertisements

, , ,

  1. #1 by arikfr on April 24, 2012 - 09:52

    I agree with that it’s better to use metaprogramming than writing redundant, less maintainable code. What I’m not sure about is why not use the scope method?

    STATUS.each do |k, v|
    scope k, where(:status => v)
    end

    Seems even simpler than defining the singleton methods.

    • #2 by sygendev on April 24, 2012 - 10:29

      1) It is a personal taste issue.
      2) I was influenced by this post: http://www.railway.at/2010/03/09/named-scopes-are-dead/

      • #3 by arikfr on April 26, 2012 - 12:01

        At first I wanted to suggest using scope for simple scopes (like the example) and use singleton methods for the more hairy ones. But this might create confusion for other collaborators of the same code, so I guess you better pick one method and as you said – it’s a personal taste…

  2. #4 by Jon Kern (@JonKernPA) on April 24, 2012 - 15:29

    Though it seems cleaner, my concern would be if it makes the class methods harder to discern at first glance.

    Four lines of clear code that show 4 methods would get my vote vs four lines of obfuscated code, hiding what the methods of the class are.

    BTW: I use this technique frequently in (boring) specs, for example when testing a rendered HTML page that has certain sections that should only appear when there is data present:

    dynamic_sections.each do |s|
    it(“does not have: #{s}”) { html.should_not =~ /#{s}/ }
    end

    This saved 24 lines of “it” specs.

  3. #5 by sygendev on April 24, 2012 - 17:50

    Thanks for the cite!

  1. DRY your RSpecs » Technical Debt

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: