patterncMinor
Better way to write this branch with an exceptional retry in an embedded C environment?
Viewed 0 times
thiswithembeddedwaywritebetterenvironmentexceptionalbranchretry
Problem
A while back I wrote the following code (target is embedded C code on an 8 bit atmel device):
I rarely rarely use a goto. Sometimes in embedded land, I talk myself into it. I'm aware that goto is bad, let's skip that issue. The real goal of the code flow was to capture 3 cases:
Then we refacatored the
We came up with:
This is terse, but I'm not a fan of meaningless
A more explicit version is
But this still includes the
```
do
{
if (!radioReceive(radiogram, MatchTree, CycleClock, duration))
bool received;
tryAgain:
received = radioReceive(radiogram, MatchTree, CycleClock, duration);
if (received && radiogram->timeSlit == SolicitMask)
goto tryAgain; // not interested in recursion and the (stack) problems that could come with it
return received;I rarely rarely use a goto. Sometimes in embedded land, I talk myself into it. I'm aware that goto is bad, let's skip that issue. The real goal of the code flow was to capture 3 cases:
- the
radioReceivetimed out, so we pass thefalseback up
- it returned, but what was heard was basically a false positive, so try some more (this is somewhat exceptional)
- otherwise it was good and we return true
Then we refacatored the
timeSlit test to include some other conditions that we felt were false positives (new function named radiogramIsValidSync()), but we got the logic backwards. At which point, we concluded this code was screwy and should be rewritten. But we diverged on how to best express those 3 conditions.We came up with:
do {
bool received = radioReceive(radiogram, MatchTree, CycleClock, duration);
if (received == false) return false;
else if (radiogramIsValidForSync(radiogram)) return true;
} while (1);This is terse, but I'm not a fan of meaningless
while()'s (would rather use empty for conditions). I also don't care for the fact that 2 of the 3 conditions are explicit, but the other is implicit.A more explicit version is
bool received
tryAgain:
received = radioReceive(radiogram, MatchTree, CycleClock, duration);
if (received)
{
if (radiogramIsValidForSync(radiogram))
return true;
else
goto tryAgain;
}
else
{
return false;
}But this still includes the
goto. A terse version is:```
do
{
if (!radioReceive(radiogram, MatchTree, CycleClock, duration))
Solution
If you can do without the
The only real drawback is that it gets a bit awkward if you need to do anything at both exit points, other than return.
received variable...just make the radioReceive(...) the condition of a while loop. You get rid of received, and your loop condition now has a purpose. :)while (radioReceive(radiogram, MatchTree, CycleClock, duration)) {
if (radiogramIsValidForSync(radiogram)) return true;
}
return false;The only real drawback is that it gets a bit awkward if you need to do anything at both exit points, other than return.
Code Snippets
while (radioReceive(radiogram, MatchTree, CycleClock, duration)) {
if (radiogramIsValidForSync(radiogram)) return true;
}
return false;Context
StackExchange Code Review Q#39757, answer score: 5
Revisions (0)
No revisions yet.