I don't know the answer to your question but I suggest that to make your code clearer you put each curly bracket on its own line so that the conditional code blocks are clearer
if (bDataReady)
{
bDataReady = false;
ADXL355Data d;
accel.readData(d);
if ((abs(d.x) < 128000) and (abs(d.y) < 128000))
{ // max 1G
if ((period_us > 0) and (rpm > (TargetRPM - TargetRPMTolerance)) and (rpm < (TargetRPM + TargetRPMTolerance)))
{
...
}
Interesting that the compiler is able to optimize to that degree.
I guess I should have posted the whole procedure. In this particular procedure there is no else statements. In other words if any of the (nested) if statements fails all the rest of the code is skipped. That means I could replace each of the conditions connected with an "and" with an if statement (four additional if's and four and's removed). That made me wonder which way to go. Does the compiled machine code stop evaluating subsequent conditions if the first condition fails if followed by an and? How much extra machine code does an if statement produce?
Here's the whole procedure:
void loop() { //executes on core 1 by default
if (bDataReady) {
bDataReady = false;
ADXL355Data d;
accel.readData(d);
if ((abs(d.x) < 128000) and (abs(d.y) < 128000)) {// max 1G
if ((period_us > 0) and (rpm > (TargetRPM - TargetRPMTolerance)) and (rpm < (TargetRPM + TargetRPMTolerance))){
//add values to one of MAX_SAMPLES slots in rotation period
int32_t delta = sample_time_us - last_tach_time_us;
if ((delta > -1) and (delta < period_us)){
int index = (MAX_SAMPLES * delta) / period_us;
//if ((index < MAX_SAMPLES) and (index > -1)) { //not sure this is needed after above checks period>0, delta<period
sample_buffer[index].NumberOfSamples =+ 1;
sample_buffer[index].x =+ d.x;
sample_buffer[index].y =+ d.y;
//}
}
//location in buffer with highest value should be angle location
}
}
}
}
As already stated in my original post, returns won't work here because it's in void loop() { . At least I don't think so. What would it return to?
Yes would work in a separate procedure called from void loop() { .
Odd that compiler didn't complain about my =+. Now I need to look up the proper syntax.
Edit: Thanks for catching that! Definitely a bug. The + is interpreted as unitary operator and array value simply replaced rather than being added to. Will be interesting to see the results now.
As a general note, adding all those extra parentheses around each condition makes the code harder for readers: they have to more carefully check whether you're doing anything "tricky" that requires them.
But most loop functions do in fact finish. Even without a return, they get to the end. What happens then?
To answer your general question, the compiler is generating tests and jumps. For a chain of and conditions, It might be roughly
test first
jump-otherwise done1
test second
jump-otherwise done1
test third
jump-otherwise done1
; tests all pass
do-the-thing
done1:
; whatever is next
Most languages do short-circuit evaluation of boolean expressions. For a chain-of-and, it stops at the first false, because the rest can't switch the result back to true. This means that if any of those subsequent tests have side effects -- intentionally or not -- they don't happen. For chain-of-or, it stops at the first true.
So nestedif and chain-of-and have the same effect.
Code should be read-able for humans and parse-able for compiler
So
Only optimize for speed if code is not fast enough
Only optimize for size if it does not fit in memory
Comment the optimizations if needed.
You have five conditions that all need to be true,
Order them so the one that failed most often is tested first
That would save a few clock-cycles on average which adds up.
For most applications it won't make a difference, for some conditions that are tested hundreds or thousands times per second it adds up.
I disagree. If nicely formatted (like in post #6), parenthesis are fine. And preferred. There is a significant lack of clarity when it comes to rules of precedence in complex conditional statements.
// -----------------------------------------------------------------------------
void myTest()
{
if (! bDataReady)
return;
bDataReady = false;
ADXL355Data d;
accel.readData(d);
if ((abs(d.x) < 128000) and (abs(d.y) < 128000)
and (period_us > 0)
and (rpm > (TargetRPM - TargetRPMTolerance))
and (rpm < (TargetRPM + TargetRPMTolerance))) {
// add values to one of MAX_SAMPLES slots in rotation period
int32_t delta = sample_time_us - last_tach_time_us;
if ((delta > -1) and (delta < period_us)) {
int index = (MAX_SAMPLES * delta) / period_us;
// not sure this is needed after above checks period>0,
// delta<period
// if ((index < MAX_SAMPLES) and (index > -1)) {
sample_buffer[index].x =+ d.x;
// }
}
// location in buffer with highest value should be angle location
}
}
// -----------------------------------------------------------------------------
void loop()
{
myTest ();
}
It's more readable if one is familiar with abs(). It shows up in exzctly this circumstance often enough. Subtraction measures distance, the expression asks if rpm is further away from the target than a tolerance. The abs turns a vector into a scalar.
First, and mostly, it's less ink. Also, it's In the spirit of a stated benefit of the assignment operator
x += k;
K&R The C Programming Language :
Their example is contrived, I hope, but makes the point.
Thank you. That answers it and in favor of putting as many (and) conditions in an if statement as possible given that nested if statements makes it harder to read (longer lines).