We're implementing a service for creating a subscription where the subscription may be linked to an order. Quick background: the purpose of this service is to allow the customer to receive the item she's subscribing to as early as possible. The solution we chose is to create a separate order for the first item before her "real" subscription starts.
Here's a snippet of the service class. I shortened the name of the
directive method to remove extraneous details from the snippet:
class Subscription belongs_to :order end class SubscriptionCreator def call # ... return directive if directive.is_a?(String) # ... subscription = create_subscription(order: directive) # ... end private def create_subscription(order: nil) subscription = Subscription.new # ... subscription.order = order if order subscription.save! subscription end def directive # Return values: # * Successful: the order created. # * Error: An error message string. # * Not applicable: nil. end end
Here are the main parts of the service:
SubscriptionCreator#callis the entry point.
SubscriptionCreator#directivecreates the order if applicable.
SubscriptionCreator#create_subscriptioncreates the subscription and ties it to the order if one was created.
I felt bothered about the
order argument of
create_subscription can directly call the
directive method. So I proposed this change:
def call # ... return directive if directive.is_a?(String) # ... # No longer has order argument. subscription = create_subscription # ... end def create_subscription subscription = Subscription.new # ... subscription.order = directive if directive subscription.save! subscription end
However, the author of the service prefers the original code. He argues that it's best if
create_subscription doesn't know the logic for creating the order, that is, it doesn't know about the
directive method. That logic he says should belong in the
He argues that with the original code, it would be easier to move
create_subscription somewhere else because it would contain no domain specific logic. He adds that keeping the lower level layers free from domain logic would make methods more modular and therefore more reusable and easier to extract.
On my part, I think that removing the argument ensures that
create_subscription only has one source of information for the created order. In the original code, there's the risk that the order passed to
create_subscription is not the same as the result of
So, what are the consequences of keeping/removing the argument from the private method?