I’m wondering if there is a pattern for developing a method which requires several sequential steps?
As an example this, not especially great, example class makes a call to a remote service, if a bunch of conditions are met first.
Whilst not a giant if/else statement, it still has a smell about it, and the class is littered with exception messages all over the place.
I’d be very grateful for suggestions or examples of best practices for this sort of requirement
class RemoteResource
def self.post(product_id)
self.new.send(:build_response, product_id)
end
private
def build_response(product_id)
product_is_present?
not_already_sent?
valid?
post_to_remote_api
end
def product_is_present?(product_id)
product.present? || raise RuntimeError
end
def product
@product ||= Product.find(product_id)
end
def not_already_sent?
@product.sent_to_remote_at.nil? || raise RuntimeError
end
def valid?
# some additional validation method
true || raise RuntimeError
end
def post_to_remote_api
# send to some remote api
true
end
end
There’s a pattern known as a Composed Method and it seems you’ve implemented that already in build_response.
I think the main problem with the current code is that it violates the Single Responsibility Principle. There is a lack of cohesion between first three steps of build_response, which are related to validation, and the last, which is about making an HTTP request.
Another thing to notice is that the vast majority of the code is behind private methods. This can make testing hard because it may require many steps to put the object into a particular state. It’s often an indication that some of the code should be extracted out into a new class.
The use of exceptions for flow control is usually a code smell, but it’s not clear from the psuedo-code you’ve posted why the validation methods are raising errors rather just returning booleans. If you can expand on that then maybe I can give a suggestion.
Andy - thanks very much for taking the time to write such a helpful response!
I think I should have either thought a lot more, or left out the example as I just sort of bashed it out in the post
Your link to the composed method pattern was very interesting and re-assuring, as was your comment about SRP - I think the most pragmatic way forward is to try and strike a balance between the two.
As for all the exceptions, well, I’ve been thinking about what’s best to return from methods when falling off the happy path, specifically avoiding returning nil, but I guess that should be the topic of another post.
