In our codebase we have a lot of controllers with a very similar structure:
@my_model.update_owner(something)
if @my_model.update_attributes(params)
return render_object(@my_model)
else
return render_validation_error(@my_model)
end
Although the structure is not exactly the same everywhere, it seems feasible to refactor. It could look like:
def update_model_and_render(model)
...
end
Certainly, we would save a lot of lines. It would also be helpful for future programmers implementing similar functionality. Calling the new refactored function would prevent them from forgetting to add the @my_model.update_owner() line.
The problem with the new function is that it would be producing a side-effect and also returning a value. Clearly, We would be breaking Command-Query Separation (the new function would be referentially opaque).
Is it ok to bend the rule in cases like this to be more DRY?.
In a lot of code that I have seen, this happens usually at the boundaries of the system (like a controller, where mutations do happen but we always return a value), but I am not sure if there is anything else I could be missing.
