I have this relatively simple function that I believe can be simpler, but I'm not entirely sure how. I'm hoping this will give me more insight as to how I can make my other functions simpler as well.
This function turns IMU roll data received (clockwise roll goes to positive 180 degrees/counterclockwise roll goes to -180 degrees) to 360 degrees of roll. Here's the function:
AWOL, current(dir) will never be more than 180 or -180.
Ok... too bad modulo doesn't work with float. What about this piece of code? I have a tolerance level. Basically, I want it to return true if the amount of roll is out of the desired roll with a specific tolerance. Is there a better way to write this?
boolean Plane::planeDisplaced(String dir, float input) {
/////// Checking To See if Plane is Within Bounderies ///////
if (current(dir) < (input - tolerance) || current(dir) > (input + tolerance)) {
return true;
}
else {
return false;
}
}
You seem to be routinely passing numeric values around as Strings (and presumably parsing them again and again each time you need to access the value). You should really consider using numbers instead of strings. You could slightly simplify your bounds check using abs(), but I don't see it making a huge difference. Get rid of those Strings, though.
The "modulo" solution makes for a short expression, but will increase execution time, because "modulo" is a relatively expensive calculation.
The "ternary operator" (x ? y:z) makes for fewer lines of code in the source, but it harder to read (IMO), and is likely to produce the same code as the if/else statement.
As other have said, your use of Strings is inappropriate and inefficient. Assuming that the argument to your function MUST be a string (ie, cleaning up this function, but not looking beyond it), you can improve things by only doing the string to number conversion once:
Thanks everyone. I have taken your advice and altered my program to use compiler variables (bytes) instead of Strings and chars. I think it will run more efficiently now. Can someone please direct me to an article (Or give me an explanation) on why using String variables are usually bad in C++/Arduino?
There is not an article about using String class in Arduino is bad yet but from experience of many members and the fact that Arduino's free() function is buggy, it is easy to conclude that keep using String class will eventually drain memory to zero and crash the program.
ZacharyBruno:
Can someone please direct me to an article (Or give me an explanation) on why using String variables are usually bad in C++/Arduino?
There is a specific problem with the Arduino String class because it exposes a bug in the Arduino memory management. But using strings to represent numbers is generally a bad idea anyway because you then need to parse the string to get the number each time you need to use it. This slows your code down and introduces potential problems in any situation where the string might not contain a valid number. Best to treat numbers as numbers - if you're given a string, parse it to a number once and use the number from then on - if you need to output a string, format the number as a string at the point you need to output it.