Is there a way to make this code more effective?

Projects General Guidance
1

I'm new to Arduino, and I'm trying to make a rather complex code. Can you help me out on fixing this ineffective and inefficient code? like using different structures or using a different type of variable.

EDIT: I'm trying to make a mode switcher, it'll change the mode when I press the button.

const int butt1   = 6;
const int butt2   = 8;
const int led1    = 2;
const int led2    = 3;
const int led3    = 5;

boolean latcher;
int mode;
boolean locker;

void setup() {

pinMode(butt1, INPUT);
pinMode(butt2, INPUT);
pinMode(led1, OUTPUT);
pinMode(led2, OUTPUT);
pinMode(led3, OUTPUT);
mode = 0;

Serial.begin(9600);
Serial.println(mode);

}

void loop() {
  
boolean  button1 = digitalRead(butt1);
boolean  button2 = digitalRead(butt2);
        
        
  if(button2 == HIGH && mode == 0 && locker == 0){
    mode = 1;
    locker = 1;
    delay(100);
      Serial.println(mode);
    }
  if(button2 == HIGH && mode == 1 && locker == 0){
    mode = 2;
    locker = 1;
    delay(100);
      Serial.println(mode);
  }
  if(button2 == HIGH && mode == 2 && locker == 0){
    mode = 3;
    locker = 1;
    delay(100);
      Serial.println(mode);
  }
  if(button2 == HIGH && mode == 3 && locker == 0){
    mode = 4;
    locker = 1;
    delay(100);
      Serial.println(mode);
  }
  if(button2 == HIGH && mode == 4 && locker == 0){
    mode = 5;
    locker = 1;
    delay(100);
      Serial.println(mode);
  }
  if(button2 == HIGH && mode == 5 && locker == 0){
    mode = 0;
    locker = 1;
    delay(100);
      Serial.println(mode);
  }
  
//looping the menu
  if(mode > 6 && locker == 0){
    mode = 0;
    delay(10);
      Serial.println(mode);
  }
     
//detect the button when released      
  if(button2 == LOW){
    if(mode >= 0 && locker > 0){
      locker = 0;
      delay(100);     
    }
  }   


//mode menu
if(mode == 0){
  digitalWrite(led1, LOW);
  digitalWrite(led2, LOW);
  digitalWrite(led3, LOW);
  }
if(mode == 1){
  digitalWrite(led1, HIGH);
  digitalWrite(led2, LOW);
  digitalWrite(led3, LOW);
  }
if(mode == 2){
  digitalWrite(led1, LOW);
  digitalWrite(led2, HIGH);
  digitalWrite(led3, LOW);
  }
if(mode == 3){
  digitalWrite(led1, LOW);
  digitalWrite(led2, LOW);
  digitalWrite(led3, HIGH);
  }
if(mode == 4){
  digitalWrite(led1, HIGH);
  digitalWrite(led2, HIGH);
  digitalWrite(led3, LOW); 
  }
if(mode == 5){
  digitalWrite(led1, LOW);
  digitalWrite(led2, HIGH);
  digitalWrite(led3, HIGH); 
  }
}

thank you!

2

Can you help me out on fixing this ineffective and inefficient code?

"Ineffective" means it's not producing the desired effect. If that's the case you'll need to describe what it's (not) doing compared to what you want it to do.

Or is it working correctly (ie it's effective) and you want it to be more efficient, only?

(If it's already doing what you want, it can't be more effective than that.)

3

kenwood120s:
"Ineffective" means it's not producing the desired effect. If that's the case you'll need to describe what it's (not) doing compared to what you want it to do.

Or is it working correctly (ie it's effective) and you want it to be more efficient, only?

(If it's already doing what you want, it can't be more effective than that.)

sorry for using wrong a wrong word, I mean how to make the code more EFFICIENT than what I already made.

4

I don't know if it would be more efficient, but better practice is that when you have code that is repeated with just values changing, put he code into a function.

if(mode == 0){
  digitalWrite(led1, LOW);
  digitalWrite(led2, LOW);
  digitalWrite(led3, LOW);
  }

The above code is repeated over and over with just the values changing, one could make a function like this:.

void digOut(boolean led1State, boolean led2State, boolean led3State)
{
  digitalWrite(led1, led1State);
  digitalWrite(led2, led2State);
  digitalWrite(led3, led3State);
}

Then call the function with:

if(mode == 0)
{
  digOut(LOW, LOW, LOW);
}
or 
if(mode == 5)
{
  digOut(LOW, HIGH, HIGH);
}
5

This seems to do absolutely nothing in your current scheme:

//looping the menu
  if(mode > 6 && locker == 0){
    mode = 0;
    delay(10);
      Serial.println(mode);
  }

mode is a state variable that is constrained to 0-5 by your state machine. It would actually behoove you to encode the mode choices as a switch statement.