raquelalegre
commented
9 years ago In CS_comm.ino:
As is now:
int CS_checkresponse(String Str_exp) {
//compare intput string and string in input buffer
int respflag = 0;
/*
Serial.print("This just in ... ");
Serial.println(CS_inputBuffer);
Serial.print("expected ");
Serial.println(Str_exp);
*/
if (Str_exp == CS_inputBuffer)
{
// Serial.println("they match");
respflag = 1;
}
else
{
// Serial.println("they dont match");
respflag = 0;
}
return respflag;
}Could be something like:
/*
* Compares given string and CS input buffer.
* @param input Whatever this argument represents
* @return Strings match
*/
boolean CS_CheckResponse(String response) {
return response == CS_InputBuffer;
}- That notation in the first comment can be understood by tools like Doxygen that'll build automatically documentation for your C code.
- In several parts of the code, I see functions returning ints and variables used as flags declared as ints. If they can only take 0 or 1 values, they'd better be booleans: makes things more readable, more safe (if you declare them as ints, nothing stops me from giving them somewhere else in the code a -2342 value and break things), and take less space.
- In general, I see many variables being global, like CS_inputBuffer. It's good practice to avoid them, because there's side effects that can happen and you might not like. Making them local make it easier to debug, as well as frees space once they are not needed. It doesn't mean you can't use them when reasonable, though.
- Variable/Function naming may need reviewing.
- Debug needs to be done differently, as we'll discuss in #21
- All this function does is compare two strings, might not need a separate function after all.
Jimbles
I've detected a few things that can be simplified in the Arduino code to make it more readable and/or efficient. I thought I could list some of them in this issue as I run into them, we can then replace them together and hope you keep them in mind when you develop in the future.