What is your way to decoupling your classes?

I was doing some app, and there, if one model was updated it had some pretty much domino update effect on many other models.

Firstly I’ve just called the needed classes right in needed method. But that was ugly, and I’ve tried to decouple them with pub/sub.

But actually that doesn’t solve the problem much either, you anyway have to subscribe needed objects and broadcast from method so it’s a bit better but it’s not far from first approach. Rails callbacks are pretty good for that task, but it’s suitable not for all cases and not explicit on what happens.

So I’ve just made services whose sole responsibility was managing classes on events, and in models 've just added bunch of methods like on_some_event. So I’m just calling that managing class, and it does it’s job, like:

class Contract
  def approve(by_user)
     run_approval_procedures(by_user)
     MysteriousManaginService.on_contract_approval(self)

and than in that class im just calling needed models and etc like

class MysteriousManaginService
    class << self
      def on_contract_approval(contract)
        UserNotification.on_contract_approval(contract.id)
        contract.executor.on_contract_approval(contract.approvers)
        #other things that need to be done

yeah that class knows too many things itself, but it is it’s responsibility to do so, and at least you easilly controll the flow and main players are not knowing each other directly. And in case something is changing, you just open that class and know what and where to change things.

So is it architectually wrong approach? What do you do for decoupling in that cases?

2 Likes

By the looks of things it is Contract that is being handed forward in each situation. I would have the Contract methods return self. And other class methods that require Contract object properties should either be designed to receive a Contract itself (as a DuckType) or have wrapper methods to take the Contract and call the appropriate methods on its data.

class Contract
  def approve(by_user)
     run_approval_procedures(by_user)
     self

Returning self from methods allows chaining multiple actions onto the same Object. So you can do things like: contract.process.send.record_to_log

module MysteriousManaginService
  module_function

  def approval_process(contract)
    user_notify(contract)
    executor_approvers(contract)
    #other things that need to be done
  end

  # consider the following methods private (even though they're not)
  def user_notify(contract)
    UserNotification.on_contract_approval(contract.id)
  end

  def executor_approvers(contract)
    contract.executor.on_contract_approval(contract.approvers)
  end
end

This would be my first refactoring. on_contract_approval (renamed to approval_process) doesn’t need to know anything and doesn’t have any coupling. Each complex action should be in a simple method wrapper. Ideally those should be private methods and the public methods will be simple and uniform.

If the Contract will always be handled by the same mysterious managing service then I would add that to the contract as an instance variable.

class Contract
  def initialize(managing_service = DefaultManagingService)
    @managing_service = managing_service
  end

  def manage_contract
    @managing_service.approval_process(self)
    self
  end
end

And use it:

contract = Contract.new(MysteriousManaginService)
contract.approve.manage_contract

You can put a check in the beginning of the manage_contract method to verify approval first.

 #silently
 return self unless @approved

 # or loudly
 raise "unapproved!" unless @approved

OR

You can create a whole new Object.

class Contract
  attr_reader :managing_service
  def initialize(managing_service = DefaultManagingService)
    @managing_service = managing_service
  end

  def approve(by_user)
     run_approval_procedures(by_user)
     ApprovedContract.new(self)
  end
end

class ApprovedContract
  def initialize(contract)
    @contract = contract 
  end

  def manage_contract
    @contract.managing_service.approval_process(@contract)
    self
  end
end

And still run:

contract = Contract.new(MysteriousManaginService)
contract.approve.manage_contract

Here approve returns an instance of ApprovedContract. And all the important methods are on ApprovedContract so you won’t accidentally run managing processes on unapproved contracts. Since you seemed to imply the importance of approval with having on_contract_approval methods everywhere.

1 Like