I have a function (postData) which posts data to a google sheet. It works great most of the time, and when it works, it returns the value "2". Every once and a while the internet appears to be misaligned or something and it doesn't work, in which case the function returns the value "0".
In the event the post doesn't go through the first time, I'd like my program to immediately try again (and it almost always succeeds on the second try, but if not, I'd like it to keep trying until it gets a "2").
Is there a better (cleaner) way to do this then my current approach?
while ((postData("Periodic", temp, vacuum, tank)) == 0) {
;
}
In case it effects things, I'll quote my postData function here (I'm already bracing for the criticism about using all these strings, but I can't figure out a better way to make it work- I'd be glad to listen.)
int postData(String myStatus, int temp, int vacuum, int tank)
{
Serial.println ("Arrived at postData");
ESP8266Client client;
Serial.println ("Finished creating client");
// ESP8266Client connect([server], [port]) is used to
// connect to a server (const char * or IPAddress) on
// a specified port.
// Returns: 1 on success, 2 on already connected,
// negative on fail (-1=TIMEOUT, -3=FAIL).
Serial.println ("Calling client connect");
int retVal = client.connect(destServer, 80);
Serial.print ("Return Value is: ");
Serial.println (retVal);
if (retVal <= 0)
{
Serial.println(F("Failed to connect to server."));
return;
}
Serial.println("Writing to Client: ");
Serial.print(startHttpRequest);
client.print(startHttpRequest);
Serial.print(labelStatus);
client.print(labelStatus);
Serial.print(myStatus);
client.print(myStatus);
Serial.print(labelTemp);
client.print(labelTemp);
Serial.print(temp);
client.print(temp);
Serial.print(labelVac);
client.print(labelVac);
Serial.print(vacuum);
client.print(vacuum);
Serial.print(labelTank);
client.print(labelTank);
Serial.print(tank);
client.print(tank);
Serial.print(endHttpRequest);
client.print(endHttpRequest);
Serial.println("Number of Characters: ");
int numberOfCharacters = (client.available());
// Serial.print(client.available());
Serial.print(numberOfCharacters);
// available() will return the number of characters
// currently in the receive buffer.
while (client.available())
Serial.write(client.read()); // read() gets the FIFO char
// connected() is a boolean return value - 1 if the
// connection is active, 0 if it's closed.
if (client.connected())
client.stop(); // stop() closes a TCP connection.
return numberOfCharacters;
}
Thank you for your help and comments. I'm 100% self (google) taught, so it wouldn't surprise me if I have some bad habits that need fixing.
while ((postData("Periodic", temp, vacuum, tank)) == 0) {
;
}
That gets it in a single line. I can't imagine how you're going to get any more cleaner than that. What feels unclean about it?
It doesn't really feel like a single line- it feels like 3 lines (at least when I look at it on my page). Before I implemented the error checking I was able to do it in one line:
postData("periodic",temp, vacuum, tank);
Now it takes 3- I was hoping there was a test that would make it fit again on a single line (just for my clarity of reading the code). Maybe not?
Knock yourself out. The structure is frowned upon in some circles. If you mistakenly leave out the ';', it's still valid code that will compile and run. But, the results will be very different. Tough to debug. Personally, I'd compromise at 2 lines:
while ((postData("Periodic", temp, vacuum, tank)) != 2) {
}
gfvalvo:
Knock yourself out. The structure is frowned upon in some circles. If you mistakenly leave out the ';', it's still valid code that will compile and run. But, the results will be very different. Tough to debug. Personally, I'd compromise at 2 lines:
while ((postData("Periodic", temp, vacuum, tank)) != 2) {
}
Honestly, the last thing I need to do is pick up a bad habit. I'll go with two lines, which will still be easier to read than 3 (although I'm awful tempted by the one line).
Also, in answer to a previous question of yours, the "return" when it failed to connect to the server was from a previous version of the code (actually, from the version I stole from the demo that came with the library). That was before I knew I needed to look for a response from the server to know that the post went through successfully. So, I should probably have it return "0" Can I just write this?
Delta_G:
If you really just think that less lines means cleaner code then why not write the function like this:
int postData(String myStatus, int temp, int vacuum, int tank){ Serial.println ("Arrived at postData");ESP8266Client client;Serial.println ("Finished creating client");Serial.println ("Calling client connect");int retVal = client.connect(destServer, 80);Serial.print ("Return Value is: ");Serial.println (retVal);{return;}Serial.println("Writing to Client:");Serial.print(startHttpRequest);client.print(startHttpRequest);Serial.print(labelStatus);client.print(labelStatus);Serial.print(myStatus);client.print(myStatus);Serial.print(labelTemp);client.print(labelTemp);Serial.print(temp);client.print(temp);Serial.print(labelVac);client.print(labelVac);client.print(vacuum);Serial.print(labelTank);client.print(labelTank);Serial.print(tank);client.print(tank);Serial.print(endHttpRequest);client.print(endHttpRequest);Serial.println("Number of Characters: ");int numberOfCharacters = (client.available());Serial.print(numberOfCharacters);while (client.available())Serial.write(client.read());if (client.connected())client.stop();return numberOfCharacters;}
Do you consider that clean? It is now, by your definition, a one-liner.
Obviously most things are ridiculous if carried to the extreme ("You like chocolate? Then why not make your entire diet consist of chocolate? If chocolate is good, then it must be good if it's your entire diet!")
But thanks for your comments.
In all seriousness, when reading the code it seems to me that doing one "thing" (like a function call) should be on one "line" of code. I guess what I still am getting my head around is when (if ever) a carriage return matters to the compiler. If I can put the function call on one line that makes sense. Your two examples don't, because one is doing one "thing" on multiple lines, and the other is doing multiple things on one line. What I was really asking for was a way to eliminate the extra spaces and superfluous braces, which is what the other helpful poster did. My goal here was to make it easy for me to come back to in a year and read through quickly if I need to modify it.
format is the easiest format to read, understand and modify
The only problem I was having wrapping my head around is that the thing I'm doing is also the logic test- the function call returns the value of whether it was successful...
jstamp:
Obviously most things are ridiculous if carried to the extreme ("You like chocolate? Then why not make your entire diet consist of chocolate? If chocolate is good, then it must be good if it's your entire diet!")
But thanks for your comments.
In all seriousness, when reading the code it seems to me that doing one "thing" (like a function call) should be on one "line" of code. I guess what I still am getting my head around is when (if ever) a carriage return matters to the compiler. If I can put the function call on one line that makes sense. Your two examples don't, because one is doing one "thing" on multiple lines, and the other is doing multiple things on one line. What I was really asking for was a way to eliminate the extra spaces and superfluous braces, which is what the other helpful poster did. My goal here was to make it easy for me to come back to in a year and read through quickly if I need to modify it.
But you are doing two things here. You're calling the function, and also looping back to retry if you don't get the right return value. That is two things, logically.
Good coding style always involves finding the correct balance that maximizes comprehension and minimizes the chance and effect of errors. This is one reason many people will prefer using curly braces for loop statements like this even when they are not necessary. If there are no braces, the very next statement is considered the body of the loop. The semicolon in the one liner version posted here creates an empty statement that does nothing. If you forget it, your next line of code will be put under the loop. Adding the empty curly braces make it blindingly obvious that there is no body to the loop. If you make it your habit to always use curly braces, it's much harder to make a mistake or suffer from ambiguity.
Which if statement is that else actually attached to? Indentation suggests that it is the alternative to condition_A, but is that actually true? Syntactically it could be interpreted both ways. A good compiler will warn you about the ambiguity, but it is perfectly legal code. I actually don't know the answer off hand, because this is one of those esoteric situations you don't run into very often, so it's very unlikely to be memorized.
Adding curly braces will add a few extra lines to this, but will make it unambiguous which statements are associated with each if statement.
