 # looking for advice on streamlining a function SOLVED.

Hello. I have a function that averages 3 values and knows how to do it depending on which values are greater than zero:

``````int8_t average(int8_t near, int8_t far, int8_t out) { //returns avg temp and knows how to avg temps depending on which sensors are plugged in.
if (near < 0 && far < 0 && out < 0) { //if none are plugged in, return zero.
return 0;
} //end if.
else if (near < 0 && far < 0) { //if two of the 3 are unplugged, return the one that isnt.
return out;
} //end else.
else if (far < 0 && out < 0) {
return near;
} //end else.
else if (near < 0 && out < 0) {
return far;
} //end else.
else if (near < 0) { //if one is unplugged, return avg of other two.
return (far + out) / 2;
} //end else.
else if (far < 0) {
return (near + out) / 2;
} //end else.
else if (out < 0) {
return (near + far) / 2;
} //end else.
else { //return avg of all 3.
return (near + far + out) / 3;
} //end else.
} //end average().
``````

It seems like its doing the same tests over and over and probably redundantly. Any ideas on how to streamline it?

Thanks!

Try counting the number of 0 values. then subtract that from 3. your "average" is then just sub of values/(3-num of 0). BUT remember to avoid division by 0!

Mark

Thanks. Ill think about it. Only Chuck Norris can divide by zero.

The way you programmed it, is good.
I would to that in a different way, with a second function ‘average’.

``````  if (near >= 0 && far >= 0 && out >= 0)
{
// everything is valid, calculate the average.
avg = average( near, far, out);
}
else
{
// something is wrong.
....           <-- all the if .. else go here.
}
``````

I didn’t try to think through what holmes4 wrote, but precalculations with afterwards correction should work.
Perhaps there a few variations for that.

If the temperature is 50 degrees, the function is wrong. (50 + 50 + 50) / 3 will not fit in an 8 bit signed.

how about something like this -

``````total=0;
count=3;

if (near<0) count--; else total+=near;
if (far<0) count--; else total+=far;
if (out<0) count--; else total+=out;

``````

If the temperature is 50 degrees, the function is wrong. (50 + 50 + 50) / 3 will not fit in an 8 bit signed.

Its worked fine so far and my average temp is 62. I think the compiler is smart enough to not assign the value until after its been divided.

``````if (out<0) count--; else total+=out;
``````

No ;before the else dude!

Mark

It would be interesting to know what assembly code the compiler will make of that. An 8-bit microcontroller that does a calculation in 16 bits with variables that are 8 bit ? I would not expect that. As far as I know that calculation is wrong with 62 degrees.

apologies vulture please disregard my suggestion as it contained an extra semicolon. best stick to your original code. =(

I’d call this somewhat streamlined. Plug some values into the three variables and give 'em a try.

``````#define near 0
#define far 1
#define out 2
int temps;

void setup() {
Serial.begin(115200);
temps[near] = 50;
temps[far] = 40;
temps[out] = 0;
Serial.println(average());
}

int average() {
int avg = 0;
int count = 0;
for (int i = 0; i<3; i++) {
if (temps[i] > 0 ) {
count++;
avg = avg + temps[i];
}
}
if (avg > 0) {
avg = avg / count;
}
return avg;
}

void loop() {
}
``````

Nice, ill try that!

But do you mean `avg = avg / count;`?

vulture2600: But do you mean `avg = avg / count;`?

Well,it does work better that way. :cold_sweat:

this seems to be working :

``````int8_t average(int8_t near, int8_t far, int8_t out) {
uint8_t count = 0;
uint16_t avg = 0;
if (near > 0) {
count ++;
avg += near;
}
if (far > 0) {
count ++;
avg += far;
}
if (out > 0) {
count ++;
avg += out;
}
if (count != 0) {
return avg / count;
}
else return 0;
} //end average().
``````

thanks for the help, everyone!

Very nice code lar3ry, you should be a teacher. I like that code, since it can be extended for more values easily.

I would change this : `if (avg > 0) {` To this: `if (count > 0) {` Because that is closer to the actual funtionality.

And I would use "`avg +=`" and "`avg /=`"

vulture2600, your resulting code is so easy now. I didn't know that it was possible. Great result. The compiler has to convert 8-bit variables to 16-bit integers and vice versa, that requires more code and is slower than using 16-bit integers. I learned that by trying to optimize my sketch by using 8-bit variables, but the mix of 16-bit and 8-bit made it slower.

holmes4:

``````if (out<0) count--; else total+=out;
``````

No ;before the else dude!

Mark

If that code were written properly:

``````if(out < 0)
count--;
else
total += out;
``````

Which ; do you think doesn’t belong?

``````if (i want to put an if...then..else statement on one line)
{
i can;
it does not mean the code is not written "properly";
it is personal preference && pragmatic choice acquired through many years programming experience;
i need to include a ";" before the else keyword, as i indeed did;
}
else
{
i can make a trivial statement 4 lines long;
}
``````

Note how I could have said that in one sentence but thought it would be more readable to format it properly :)

it is personal preference && pragmatic choice acquired through many years programming experience;

As long as you are the only one maintaining the code, do what you want. If you want other people to help, do it right.

``````if (out<0) count--; else total+=out;
``````

As there are two semi colons, you have two statements. Might as well do it properly and keep it to a single statement per line, however that is also my preference.

``````total += (out < 0) ? ++count, 0 : out;
``````

Paul are you seriously saying the code below would be hard to maintain???

``````if (out<0) count--; else total+=out;
``````

I think you’re taking the concept of general programming good practice (which I would not disagree with) and then using it to argue the most trivial of real-life cases. Really quite unnecessary within the context of this thread, which if you remember was about simplifying an algorithm.

My comments were initially directed at holme4, who said that a semicolon was unnecessary. I disagreed.

I do not like having more that one statement on a line. You have 4 on one line. Yes, that makes it harder to maintain code. What if you have some issue with that block of code, and need to print a variable’s contents? It is, IMHO, a lot harder to add a print statement to:

``````if (out<0) count--; else total+=out;
``````

then it is to add one to:

``````if(out < 0)
{
count--;
}
else
{
total += out;
}
``````

While the latter does take more vertical space to display, the intent and structure is far easier to see, in my opinion.