You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I am currently trying to add functionality to parts of you library. Doing that, i constantly run into endles if-else chains big enough for the code to go off the right side of my screen...
These almost always are just checking if something succeeded, and if not exiting the function with an error. This can be written more cleanly like this: if (notFoundOK()) return "ERROR";. If you are just planning to return an error, this has the same effect. As soon as a return is executed, the function exits and code following it will not be executed.
Here is an example:
GSMSimHTTP.cpp get(String) - original
String GSMSimHTTP::get(String url) {
if (isConnected()) {
// Terminate http connection, if it opened before! Otherwise method not run correctly.
gsm.print(F("AT+HTTPTERM\r"));
_readSerial();
gsm.print(F("AT+HTTPINIT\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) != -1) {
gsm.print(F("AT+HTTPPARA=\"CID\",1\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) != -1) {
gsm.print(F("AT+HTTPPARA=\"URL\",\""));
gsm.print(url);
gsm.print("\"\r");
_readSerial();
if (_buffer.indexOf(F("OK")) != -1) {
gsm.print(F("AT+HTTPACTION=0\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) != -1) {
_readSerial(10000);
if (_buffer.indexOf(F("+HTTPACTION: 0,")) != -1) {
String kod = _buffer.substring(_buffer.indexOf(F(","))+1, _buffer.lastIndexOf(F(",")));
String uzunluk = _buffer.substring(_buffer.lastIndexOf(F(","))+1);
String sonuc = "METHOD:GET|HTTPCODE:";
sonuc += kod;
sonuc += "|LENGTH:";
sonuc += uzunluk;
// Bağlantıyı kapat!
gsm.print(F("AT+HTTPTERM\r"));
_readSerial();
sonuc.trim();
return sonuc;
}
else {
return"HTTP_ACTION_READ_ERROR";
}
}
else {
return"HTTP_ACTION_ERROR";
}
}
else {
return"HTTP_PARAMETER_ERROR";
}
}
else {
return"HTTP_PARAMETER_ERROR";
}
}
else {
return"HTTP_INIT_ERROR";
}
}
else {
return"GPRS_NOT_CONNECTED";
}
}
GSMSimHTTP.cpp get(String) - modified
String GSMSimHTTP::get(String url) {
if (!isConnected()) return"GPRS_NOT_CONNECTED";
// Terminate http connection, if it opened before! Otherwise method not run correctly.
gsm.print(F("AT+HTTPTERM\r"));
_readSerial();
gsm.print(F("AT+HTTPINIT\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) != -1) return"HTTP_INIT_ERROR";
gsm.print(F("AT+HTTPPARA=\"CID\",1\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) == -1) return"HTTP_PARAMETER_ERROR";
gsm.print(F("AT+HTTPPARA=\"URL\",\""));
gsm.print(url);
gsm.print("\"\r");
_readSerial();
if (_buffer.indexOf(F("OK")) == -1) return"HTTP_PARAMETER_ERROR";
gsm.print(F("AT+HTTPACTION=0\r"));
_readSerial();
if (_buffer.indexOf(F("OK")) == -1) return"HTTP_ACTION_ERROR";
_readSerial(30000);
if (_buffer.indexOf(F("+HTTPACTION: 0,")) == -1) return"HTTP_ACTION_READ_ERROR";
String kod = _buffer.substring(_buffer.indexOf(F(","))+1, _buffer.lastIndexOf(F(",")));
String uzunluk = _buffer.substring(_buffer.lastIndexOf(F(","))+1);
kod.trim();
uzunluk.trim();
String sonuc = "METHOD:GET|HTTPCODE:";
sonuc += kod;
sonuc += "|LENGTH:";
sonuc += uzunluk;
// Bağlantıyı kapat!
gsm.print(F("AT+HTTPTERM\r"));
_readSerial();
sonuc.trim();
return sonuc;
}
At least in my opinion, the modified version is much easier to understand. The error message follows right after the condition that could cause it.
You could also replace all checks for OK in the buffer with one global checkOKinBuffer() defined in GSMSim.h
All that does not seem too hard to automate. A python script could probably rearrange the code automatically.
Another thing: having multiple functions with nearly-identical code and (even worse) identical strings affects the code size quite significantly. see #52
The text was updated successfully, but these errors were encountered:
I am currently trying to add functionality to parts of you library. Doing that, i constantly run into endles if-else chains big enough for the code to go off the right side of my screen...
These almost always are just checking if something succeeded, and if not exiting the function with an error. This can be written more cleanly like this:
if (notFoundOK()) return "ERROR";
. If you are just planning to return an error, this has the same effect. As soon as a return is executed, the function exits and code following it will not be executed.Here is an example:
GSMSimHTTP.cpp get(String) - original
GSMSimHTTP.cpp get(String) - modified
At least in my opinion, the modified version is much easier to understand. The error message follows right after the condition that could cause it.
You could also replace all checks for
OK
in the buffer with one globalcheckOKinBuffer()
defined in GSMSim.hAll that does not seem too hard to automate. A python script could probably rearrange the code automatically.
Another thing: having multiple functions with nearly-identical code and (even worse) identical strings affects the code size quite significantly. see #52
The text was updated successfully, but these errors were encountered: