Remove possible middle man smell is models

rubyonrails-talk
1

Hi there! I have a pretty straightforward question that maybe too simple, but it’s getting on my nerves.

Let’s imagine that I have a sequence of four models, where each has a dependency on the next.

So, we’d have something like this:

model D -> model C -> model B -> model A

Now, when something in model D changes, I’d like to also change a few things in model B and model A. However, I’d like to maintain the separation of concerns and avoid breaking Demeter’s law. Therefore I followed with this simple implementation:

class modelD
  def review!
    modelC.review!
  end
end

class modelC
  def review!
    modelB.review!
  end
end

class modelB
  def review!
    # do something else
    modelA.review!
  end
end


class modelA
  def review!
    # do something else
  end
end

Now, I know this is working but there’s probably a better way to handle this. I can see a few errors in this though:

  • modelC#review! is nothing but a delegator, so we could add a delegate to it instead.
  • If something is added to any of the intermediate methods, it’ll run every time one of its dependencies calls, even if it’s not really needed or wanted.

Looking around I found that I could use callbacks to simplify this (maybe in a Reviewable module), but that would only add a little more indirection. Another thing suggested at me was to use ActiveSupport::Notification to handle this, but I couldn’t find any example of it other than being used for logging/metrics, so I’m not so sure it’d be a great fit here too.

What do you think would be the best way to handle this?

Thanks a lot in advance and cheers from Brazil :brazil: !

2

I think I’m struggling to understand a few things. What is the nature of the dependency? An ActiveRecord::Relation? Inheritance? Or is it that each class holds a reference to the other?

If this is a method being overwritten, I’m not sure you can avoid calling any the middle men… To achieve what I think you want, I’d have each of the classes be instance variables where they’re needed. If we’re dealing with AR::Relations you kind of have that already, you’d just need some logic to skip certain steps. It wouldn’t be perfect though…

Maybe what you want is a different pattern, but it’s hard to say without knowing the specific use case…

3

Thanks a lot for the answer, @mateusdeap ! You’re absolutely right, it completely slipped my mind, but we’re talking about AR relations here. So, when I’m talking about modelB -> modelA the idea is that modelB has a belongs_to relationship to model A. Talking about patterns, I’ve had a felling that maybe the Observer pattern would fit here, thus my leaning to use ActiveSupport::Notification so far.

4

Yeah, in that case I think you’ll just have to implement the logic in each model to only call the ones that matter.

I might be mistaken, but I think what you might be looking for is a chain of responsibility

You’d still be going through every link in the chain. If the logic is simple you might be able to avoid calling each middle man, but I’m not aware of how to do it cleanly.

Using observers is definitely another possibility, depending on what your specific use case is, but you’ll have to weigh how much indirection you want to tolerate

5

Thank you, @mateusdeap. I really appreciate your suggestions.