Reputation: 1482
So I have a rather complex state machine that does multiple stuff not really relevant to my question.
Amongst those stuff is sending messages over some bus, and waiting for replies.
Depending on some of those replies, I have to send a specific message and stop sending messages.
Functions sending the messages and replies are returning some error codes I can use to determine the status of what's going on.
My issue is the following: I don't want to have much repeated code that just checks, cleans up, and returns.
Here is what it would look like (inside one of my state machine functions):
err_code_t ret_value;
ret_value = send_message_2(X, Y);
if(ret_value != ERR_CODE_OK) {
// Do some cleanup, free buffers...
return;
}
ret_value = send_message_1(Z, W);
if(ret_value != ERR_CODE_OK) {
// Do some other different cleanup, free other buffers, set some flags...
return;
}
How can I avoid repeating those same checks, that would spam my code and have it way less readable than I'd like to. It also occurs that the function send_message is called from multiple states and would need to have those checks all over the place.
What I imagined so far:
What would be my other better options? I'm looking to keep the code clean, readable, and easy to maintain. I don't care if it involves complex stuff under the hood though.
Surely this is a common thing but I didn't find anything that is as good as I'd like.
Could you suggest ways to handle those kinds of redundant error checking and recovering.
Upvotes: 0
Views: 166
Reputation: 214060
"Do Not Repeat Yourself (DRY)" is a design practice that means well in theory, but in practice can be catastrophic when someone goes out of their way to implement it, at the cost of much more important things like keeping the program simple, readable and bug-free.
Since you are already considering evil things like function-like macros to wrap clean-up code, and even considering the cardinal sin of longjmp
spaghetti, your program design is already in grave danger of a massive detail and it's caused by DRY.
Always prioritize "Keep it simple, stupid (KISS)" over DRY. Also known as using common sense, instead of vastly complicate the design just for the sake of making things complex.
First of all, error handling is tedious - it does involve a lot of checks all over the code. This is not necessarily a bad thing to begin with if we can accept that extensive error checking will lead to multiple such if
statements and clean-up code. So there's not necessarily a problem to solve here to begin with.
However, there are two concrete design improvements that I would suggest:
Separate resource allocation/clean-up from the algorithm. For example you could build a state in a state machine with an outer wrapper handling allocation. Pseudo code:
result_t some_state (void)
{
some_resource = allocate();
result_t result;
result = actual_algorithm(some_resource);
if(result == BAD)
{
cleanup(some_resource);
}
return result;
}
This leaves actual_algorithm
relatively clean and the only thing it needs to do upon errors is to return an error code.
Centralize error/result handling and state transits in one single place. It's sensible design to have a top level error handler and also making that one responsible of state transits, based on 1) which state are we in, and 2) what was the result from executing that state.
Such a design will vastly simplify the use of state machines and prevent "stateghetti" unreadable code which has state transitions all over the place. Example here.
As a bonus these changes also meant that we did use DRY, though in a controlled manner, without introducing a ton of complexity and dangerous features.
Upvotes: 1
Reputation: 754170
A limited improvement would be:
if ((ret_value = send_message_2(X, Y)) != ERR_CODE_OK ||
(ret_value = send_message_1(Z, W)) != ERR_CODE_OK)
{
// Do some cleanup, free buffers...
return;
}
This assumes that the same cleanup is needed each time — you should endeavour to make it so that the same cleanup can be used each time.
Upvotes: 1