search

apotonick / gemgem-trbrb

The Trailblazer book's example app.
http://trailblazer.to#book
137 stars 60 forks source link

OOD improvement #17

Open ghost opened 8 years ago

ghost commented 8 years ago

I like this book. Thank you for writing it. I am reading through it and came across this line:

    def reset_authorships!
      model.authorships.each { |authorship| authorship.update_attribute(:confirmed, 0) }
    end

and I thought "isn't this doing too much?" It seems to be messing around with an association's AR internals, reaching 2 levels deep. Would it not be better to have a method on the authorship form object which we write, something like

def reset_and_sync!
  confirmed = 0
  sync
end

then the code in Thing could read

    def reset_authorships!
      authorships.each(&:reset_and_sync!)
    end

Just a thought. That way the thing doesn't need to know the AR internals of an associated form object.

apotonick commented 8 years ago

That's actually a cool idea! I didn't like this code myself, but saw it as a chance to show some more internals to users. However, you're right, it might mislead them to messing around with AR internals...