I don't understand what you really want to accomplish then :D
The logic seems to be too complicated for these three lines of code.
Update 1 should be rolled back if there's an error in Communicator but the save inside communicator should happen regardless of there being an error and update 2 should happen only if communicator went along fine?
If you have control on the Communicator object you should probably keep the logic of calling the external service and remove the one that's giving you pain (which shouldn't be there in theory) which is the part about saving the error in the DB.
If the communicator only communicates you can then leave all the DB logic in your "main transaction" and decide what to do. Something like:
# update record 1
# call the service with record 1
# did the call went fine? yes? move on
# did the call raise an error? store the error in a variable
##### rollback the transaction
##### save the error outside the transaction bloc
Update 1 should be rolled back if there's an error in Communicator but the save inside communicator should happen regardless of there being an error and update 2 should happen only if communicator went along fine?
yes, exactly.
If you have control on the Communicator object you should probably keep the logic of calling the external service and remove the one that's giving you pain (which shouldn't be there in theory) which is the part about saving the error in the DB.
it is required to save the error response to the db, but maybe it could be done outside of communicator. I'll have to think about this some more.
thank you again, rhymes, for taking the time to discuss this!
it is required to save the error response to the db, but maybe it could be done outside of communicator. I'll have to think about this some more.
See it like this. The service object should be responsible for one thing only. In the future you might want to do N different things with the response. You might want to save it on the DB, you might want to log it on an external service, you might want to extract info from it. Separating the two steps should simplify the logic. In an extreme case you might even save the error later and asynchronously ;)
hmm, good thinking, but I don't think it would work :D the save error operation inside the communicator class would still be rolled back.
What do you think?
I don't understand what you really want to accomplish then :D
The logic seems to be too complicated for these three lines of code.
Update 1 should be rolled back if there's an error in
Communicator
but the save inside communicator should happen regardless of there being an error and update 2 should happen only if communicator went along fine?If you have control on the
Communicator
object you should probably keep the logic of calling the external service and remove the one that's giving you pain (which shouldn't be there in theory) which is the part about saving the error in the DB.If the communicator only communicates you can then leave all the DB logic in your "main transaction" and decide what to do. Something like:
Does it make sense?
yes, exactly.
it is required to save the error response to the db, but maybe it could be done outside of communicator. I'll have to think about this some more.
thank you again, rhymes, for taking the time to discuss this!
See it like this. The service object should be responsible for one thing only. In the future you might want to do N different things with the response. You might want to save it on the DB, you might want to log it on an external service, you might want to extract info from it. Separating the two steps should simplify the logic. In an extreme case you might even save the error later and asynchronously ;)
yeah, maybe another service could handle dealing with the response and whatever we want to do with it :D