So I need to create an OrderSynchroniser which synchronises with an external system (API). The API expects this to be done in 12 steps. Each step needs the output of the previous step as it’s input. I created something like the following. (The example is a bit simplified and only shows a few steps.)
class OrderSyncer def initialize(order) @order = order end def synchronise_all external_order_id = synchronise_order(@order) external_payment_id = synchronise_payment(external_order_id) external_contract_id = synchronise_contract(external_payment_id) synchronise_authorisisation(external_contract_id) end private def synchronise_order(order) # If the fetcher does not return an order_id we need to synchronise unless order_id = Fetcher::OrderId.new(order.id).fetch order_id = Synchroniser::Order.new(order).synchronise end end def synchronise_payment(external_order_id) #Same kind of steps as in synchronise order end def synchronise_contract(external_payment_id) #Same kind of steps as in synchronise order end def synchronise_authorisisation(external_contract_id) #last step which always synchronises and returns an id or nil end end
Or check out the gist (same as above).
The problems with this setup (imo):
- The synchronise_all is getting really big for 12 steps.
- The separate synchronise methods depend on each other being called in succession. (Don’t know if I see this as a problem or just something that I should accept).
- The OrderSynchroniser as it stands is 150 lines. Not a problem per se but thinking about changing behaviour in this class is a pain.
Solutions I thought of but fear will overcomplicate the actual code:
- Extract the separate synchronise methods to their own classes. So the synchronise_order method will be extract to the class
SynchroniseOrderwhich will Fetch OR Synchronise the order. Although this will make the
OrderSynchroniserclass smaller now I have 3 separate classes that are being called.
- Since the
synchronise_methods are all alike I thought about creating a more generic method which will be called with the classes it needs to call, making the setup some what more abstract and generic. I’m not sure this would be a step in the right direction since I’m afraid that it will make the code to abstract and hard to change in the future.
Does anybody have other ideas about setting up this code? Keep in mind that each method needs to be called in succession to each other and needs the output of the previous method.