Reduce amount of if statements

here is a piece of code that handle 4 timers and runs them in sequential “1-2-3-4” order endlessly. it has been a while since i wrote this program but now im revising it and trying to think about ways to make it less serialized. how could reduce the amount of “if statements” in this segment?

The way the code works is start by turning on timer 1 then waiting for on and period duration to expire before moving to the next timer.

I have tried to think of ways to wirte it differently but i cant really think of any other way

if (_EEPROM.timerMode == 1) {
    if (_EEPROM.enabled) {
      if (timerState == 1) {
        if (_EEPROM.timerEnabled[0]) {
          if (timerStates[0] == 0 ) { // if timer is off
            handleTimers(0, 1, 1, 1); //timer 1on, timers 2-4 off
            timerStates[0] = 1; // current timer is timer 1
            timer1onMillis = millis();
            _EEPROM.timerRuntimes[0]++;
          }
          if (lightsOn) {
            if (millis() - timer1onMillis >= _EEPROM.timerTimes[0][0] && timerStates[0] == 1) {
              handleTimers(1, 1, 1, 1); //turn off all timers
              timerStates[0] = 2; // now start timer 2
              timer1offMillis = millis();
            }
            if (millis() - timer1offMillis >= _EEPROM.timerTimes[0][1] && timerStates[0] == 2) {
              timerState = 2;
              timerStates[0] = 0;
              timer1offMillis = millis();
            }
          }
          if (!lightsOn) {
            if (millis() - timer1onMillis >= _EEPROM.timerTimes[0][2] && timerStates[0] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[0] = 2;
              timer1offMillis = millis();
            }
            if (millis() - timer1offMillis >= _EEPROM.timerTimes[0][3] && timerStates[0] == 2) {
              timerState = 2;
              timerStates[0] = 0;
              timer1offMillis = millis();
            }
          }
        } else {
          timerState = 2;
        }
      }
      if (timerState == 2) {
        if (_EEPROM.timerEnabled[1]) {
          if (timerStates[1] == 0 ) {
            handleTimers(1, 0, 1, 1);
            timerStates[1] = 1;
            timer2onMillis = millis();
            _EEPROM.timerRuntimes[1]++;
          }
          if (lightsOn) {
            if (millis() - timer2onMillis >= _EEPROM.timerTimes[1][0] && timerStates[1] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[1] = 2;
              timer2offMillis = millis();
            }
            if (millis() - timer2offMillis >= _EEPROM.timerTimes[1][1] && timerStates[1] == 2) {
              timerState = 3;
              timerStates[1] = 0;
              timer2offMillis = millis();
            }
          }
          if (!lightsOn) {
            if (millis() - timer2onMillis >= _EEPROM.timerTimes[1][2] && timerStates[1] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[1] = 2;
              timer2offMillis = millis();
            }
            if (millis() - timer2offMillis >= _EEPROM.timerTimes[1][3] && timerStates[1] == 2) {
              timerState = 3;
              timerStates[1] = 0;
              timer2offMillis = millis();
            }
          }
        } else {
          timerState = 3;
        }
      }
      if (timerState == 3) {
        if (_EEPROM.timerEnabled[2]) {
          if (timerStates[2] == 0 ) {
            handleTimers(1, 1, 0, 1);
            timerStates[2] = 1;
            timer3onMillis = millis();
            _EEPROM.timerRuntimes[2]++;
          }
          if (lightsOn) {
            if (millis() - timer3onMillis >= _EEPROM.timerTimes[2][0] && timerStates[2] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[2] = 2;
              timer3offMillis = millis();
            }
            if (millis() - timer3offMillis >= _EEPROM.timerTimes[2][1] && timerStates[2] == 2) {
              timerState = 4;
              timerStates[2] = 0;
              timer3offMillis = millis();
            }
          }
          if (!lightsOn) {
            if (millis() - timer3onMillis >= _EEPROM.timerTimes[2][2] && timerStates[2] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[2] = 2;
              timer3offMillis = millis();
            }
            if (millis() - timer3offMillis >= _EEPROM.timerTimes[2][3] && timerStates[2] == 2) {
              timerState = 4;
              timerStates[2] = 0;
              timer3offMillis = millis();
            }
          }
        } else {
          timerState = 4;
        }
      }
      if (timerState == 4) {
        if (_EEPROM.timerEnabled[3]) {
          if (timerStates[3] == 0 ) {
            handleTimers(1, 1, 1, 0);
            timerStates[3] = 1;
            timer4onMillis = millis();
            _EEPROM.timerRuntimes[3]++;
          }
          if (lightsOn) {
            if (millis() - timer4onMillis >= _EEPROM.timerTimes[3][0] && timerStates[3] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[3] = 2;
              timer4offMillis = millis();
            }
            if (millis() - timer4offMillis >= _EEPROM.timerTimes[3][1] && timerStates[3] == 2) {
              timerState = 1;
              timerStates[3] = 0;
              timer4offMillis = millis();
            }
          }
          if (!lightsOn) {
            if (millis() - timer4onMillis >= _EEPROM.timerTimes[3][2] && timerStates[3] == 1) {
              handleTimers(1, 1, 1, 1);
              timerStates[3] = 2;
              timer4offMillis = millis();
            }
            if (millis() - timer4offMillis >= _EEPROM.timerTimes[3][3] && timerStates[3] == 2) {
              timerState = 1;
              timerStates[3] = 0;
              timer4offMillis = millis();
            }
          }
        } else {
          timerState = 1;
        }
      }
    }
  }

Hasn’t this been done to death in your other topic Timing sequence help - #56 by notsolowki

no totally different problem and different timing sequence. this code has not been discussed

Have a look at “switch/case”?

Here

Wouldn’t i just be replacing if statements with cases, is there some way more efficient, all of the states, array positions, timer names, millis counters are all 0-3. is there a more simple/compact way to do this?

use structs instead of multi dimensional arrays.
get rid of code duplicates.
start using object oriented programming and let each object do it’s own timekeeping.
Make interfaces for your objects and let the objects communicate via these interfaces.

I see a lot of nested commonality too…
Look at your code, and call functions once when relevant, not within every condition.

You might save 50% or more of your code size.

what do you mean by this exactly?

I was thinking more along the lines of something like this,

void handleTimer(){
  for (int a = 0; a >=3; a++){
    digitalWrite(timer(a))==LOW); //TURN ON
    if(millis()-now>= timerTimes[0]){ //on time
      digitalWrite(timer(a))==HIGH); //TURN OFF
      if(millis()-now>= timerTimes[0]){ //off time
      continue
    }
  }
}

but im not very good at these things lol. obviously continue does not do what i want it to. and the for loop is going to turn on every output right away, this is not going to work

e.g. in the topmost block…

You could put that line once - farther out in the nesting, instead of multiple times.

You may need to rearrange your nesting too - to get other optimisations.

Similar for several other statements.

I dont see how to use it any other way with my code the way it is. from what i see, lightson and lightsoff determine which off time is used. i suppose i dont need to call it after an off event

Exactly - you need to rearrange your code to use the ifs more efficiently.

but how could i rewrite this without soo many if statements and without soo many millis markers. all of my variables are in arrays inside of structs

unsigned long timer1onMillis = 0;
unsigned long timer2onMillis = 0;
unsigned long timer3onMillis = 0;
unsigned long timer4onMillis = 0;
unsigned long timer1offMillis = 0;
unsigned long timer2offMillis = 0;
unsigned long timer3offMillis = 0;
unsigned long timer4offMillis = 0;

int timerState = 1;
bool lightsOn = 1 ;

struct {
  int timerMode;
  char masterIP[20];
  bool timerEnabled[4];
  unsigned long timerRuntimes[4];
  bool timerRunning[4];
  unsigned long timerTimes[4][4]; //1-4 on,off,onN,offN
  bool enabled = true;
  char sta_ssid[15];
  char sta_password[15];
  char system_password[15];
} _EEPROM;

Why do you need 8 variables to store millis? if the timers are in sequence, you can store one “last” millis and one variable in which state you are.

Come up with a precise specification what the whole thing is about.
Describe your hardware.
Make a drawing, either a state diagramm or a flow diagram.
Afterwards we can give a proposal, otherwise we are just wasting time by following a XY problem.

i understand what your saying but i want to write that whole segment of code into 1 or 2 if statements. is that possible?

The easiest way to do this is with a series of delay()s unless the program is required to do anything else, which you have not shared with us

it is dependend on your specifications and your flow diagram.
You haven’t defined specifications, nor you have drawn a flow diagram.

Basically i see only two if

if (millis() - previousMillis > interval)
{
   // do what's needed on leaving a state
   //
   state++;
   if (state >= maxState) state = 0;
   interval = predefinedIntervals[state];
   previousMillis = millis();
   // do whats needed on entering the new state
   // 
}

i was just thinking about writing it that way just to clarify how it works. its really simple overly complicated state machine with millis(). the only time its not in sequence is if i have an output disabled then is skip that output and moves on

its main objective is to rule the world

So instead of on off millis statements just use a single if statement checking the interval. this is clear to me i think.

except 1 part,

why do you use state as array position argument. i must be missing a little bit.

EDIT: duh nvm. i will try to make something out of this thankyou

Before i go and add more if statements to handle picking the right position in timerTimes[4][4] would you be able to shed some light on this problem so i don’t start off on the wrong foot

unsigned long timerTimes[4][4]; //1-4 onDAY,offDAY,onNight,offNight

If i did it my way i would probable make the times into 4 different arrays