coding best practices

I am trying to decide what is the best practice for a conditional function. Is it better to set a variable (datcol_Frequency) and then run if statements to check if that is true, set another variable (datcol_Interval), and then run an expression like the following code:

if (datcol_Frequency == 1){
    //Set datcol_Interval to 30 minutes
    datcol_Interval = 1800000;
  }
  if (datcol_Frequency == 2){
    //Set datcol_Interval to 1 hours
    //datcol_Interval = 3600000;
   }

  if (datcol_Frequency == 3){
    //Set datcol_Interval to 2 hours
    datcol_Interval = 7200000;
  }

Then check against this expression:

[code]
if (current_datcol_Millis - previous_datcol_Millis >= datcol_Interval){
      previous_datcol_Millis = current_datcol_Millis; // save the last time you collected data

      //Start Data collection
      Serial.println("\nStarting Data Collection... ");

Or would it be better to do several of these instead:

if (datcol_Frequency ==1 ){
current_datcol_Millis - previous_datcol_Millis >= 1800000
      previous_datcol_Millis = current_datcol_Millis; // save the last time you collected data
}
      //Start Data collection
      Serial.println("\nStarting Data Collection... ");

if (datcol_Frequency ==2 ){
current_datcol_Millis - previous_datcol_Millis >= 3600000
      previous_datcol_Millis = current_datcol_Millis; // save the last time you collected data
}
      //Start Data collection
      Serial.println("\nStarting Data Collection... ");

if (datcol_Frequency ==3 ){
current_datcol_Millis - previous_datcol_Millis >= 7200000
      previous_datcol_Millis = current_datcol_Millis; // save the last time you collected data
}
      //Start Data collection
      Serial.println("\nStarting Data Collection... ");

My thinking is that the first example would use more memory, and the second would just use more program space… Any opinions on this as to which way is best?

You are new and have managed to post a question correctly with your code in code tags. If only all new folk here did that. ++Karma.

Any opinions on this as to which way is best?

My opinion is that whatever works is good, and if it's easy for me to read and understand (not so bothered about what any one else thinks) then that's good too.

Do what works for you. Go back in 6 months and you'll maybe look at your old code and realise how much you have improved.

Sorry if you were wanting me to pick one of your examples over the other but I can't.

I have thought of a different (better?) answer.

Compile your different versions, the compiler will report how much flash and RAM each uses, for example:

Sketch uses 4616 bytes (1%) of program storage space. Maximum is 253952 bytes.
Global variables use 382 bytes (4%) of dynamic memory, leaving 7810 bytes for local variables. Maximum is 8192 bytes.

I think it doesn't matter how much memory a program on a controller occupies as long as it does what it is designed to do and fits in the available space. There is some fixed amount of memory in a controller, and whether you use it or not it's all still there. What you don't waste by having a really tightly written, efficient program you waste by it sitting there doing nothing. If you don't exceed or get dangerously close to the maximum available then I don't think it matters much.

Neither of them

Put the datcol_Interval values in an array and use the datcol_Frequency value as an index to it then check against the expression

Ah, i didn't even think of using a buffer. Nice and efficient, thank you!

The problem with the first code fragment is that, if datcol_Frequency equals 1, you then perform two tests that cannot possibly be true. This lends itself to a cascading if statement block:

  if (datcol_Frequency == 1) {
     // do whatever...
  } else {
     if (datcol_Frequency == 2) {
        // do whatever this needs...
     } else {
        if (datcol_Frequency == 3) {
           // whatever...
        }
      }
   }

With this code structure, it would not test the second two conditions if the first is true. Personally, I'm not a big fan of cascading if blocks. I prefer the switch/case statement block:

  switch (datcol_Frequency ) {
      case 1:
          // do whatever is needed
         break;

      case 2:
          // do whatever is here
         break;

      case 3:
          // finally
         break;

      default:
         Serial.print("I shouldn't be here: datcol_Frequency  = ");
         Serial.println(datcol_Frequency );
         break;
   }

The switch/case results in the compiler creating a jump table, so there is only one test done to determine where the jump should be. Also, I find it a lot easier to read.

The second one is a terrible, horrible, no good, very bad idea. Never ever ever duplicate code logic like that, it's a nightmare for debugging and rework. You do it once, and you'll quickly learn to never do it again.

Jiggy-Ninja: The second one is a terrible, horrible, no good, very bad idea. Never ever ever duplicate code logic like that, it's a nightmare for debugging and rework. You do it once, and you'll quickly learn to never do it again.

You might say who you're talking to.

long datcol_Interval=1800000<<(datcol_Frequency-1);