← Back to Upcase

OO Design and Extracting Objects


(Jean Walt) #1

So I got moved onto a Rails project recently, and I see a lot of this:

def set_sub_nav
  @sub_nav_list = []

  @sub_nav_list << {
      :title => 'example',
      :slug => 'example',
      :url => gen_nav_url(routes_helper: 'example', options: {:categories => 'example'})
  }
  @sub_nav_list << {
       :title => 'example',
       :slug => 'example',
       :url => gen_nav_url(routes_helper: 'example', options: {:categories => 'example'})
   }
   @sub_nav_list << {
       :title => 'example',
       :slug => 'example',
       :url => gen_nav_url(routes_helper: 'example', options: {:categories => 'example'})
   }
 end

Pretty much in most controllers, this feels really bad…just to sanity check, this would be better extracted out into an object? I was thinking of creating a class, and allow it to be configurable on the amount of sub nav entries needed as well as the content. Is this the best way forward?

Beyond that there are also loads of objects(arrays and hashes mostly) set up in classes(mostly controllers). Often these object don’t really corrrespond to the class, like setting a Player object in the team class for instance, once again, would it be much better to extract that out into its own class?

Mostly just wanting some good suggestions, maybe some of you guys have seen a lot of this before coming onto projects where developers had not had much experience with Ruby and or Rails.

:smile:

Jean


(Joe Ferris) #2

Yeah, I’d definitely start introducing a class like NavItem for those. At the start, it will just change things like nav[:title] to nav.title, but you’ll start to see improvements. It will give you nicer failure messages like “undefined method ttle for NavItem” instead of just silently getting a nil back. Also, as you move forward, it will be easy and obvious to add methods to NavItem when you need them instead of littering the helper namespace. Those navigation hashes look like a classic case for Value Objects.


(Jean Walt) #3

Thanks @jferris, going to take a stab at refactoring it. I feel like you guys could dedicate a whole weekly iteration to Value Objects :wink: