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.