# Help simplifying nested code

**URL:** <https://forum.arduino.cc/t/help-simplifying-nested-code/1038200>\
**Category:** General Guidance\
**Created:** [October 4, 2022, 5:07am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200 "2022-10-04T05:07:22Z")\
**Posts on this page:** 17\
**Page:** 1

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 4, 2022, 5:07am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/1 "2022-10-04T05:07:22Z")

</div>

Hi guys, this is my first post to a forum and I am super excited. My project is recreating a seatbelt safety system using an Arduino, buzzer, membrane switch, and gy-521.  
The code works great but its far too complicated. I know nesting a bunch of If statements is bad practice. Do you have any suggestions of what I can do make it more simple or just improvements in general?

```cpp
#include "Wire.h"
#include "I2Cdev.h"
#include "MPU6050.h"

const uint8_t buzzer = 3; 
const uint8_t switchPin = 4;

unsigned long currentMillis;
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
unsigned int loopCount = 0;
unsigned int activated = 0;
uint8_t restartCount = 4;
uint8_t xlast = 0;
uint8_t ylast = 0;
uint8_t zlast = 0;
uint8_t previousxlast = 0;
uint8_t previousylast = 0;
uint8_t previouszlast = 0;

MPU6050 mpu;
int16_t ax, ay, az;
int16_t gx, gy, gz;

struct MyData {
  byte X;
  byte Y;
  byte Z;
};
MyData data;

void setup() {
  Serial.begin(9600);
  Wire.begin();
  mpu.initialize();
  DDRD &= ~(1 << switchPin); //set to 0 to make it an input
  PORTD |= (1 << switchPin); //set to 1 to turn on the internal pull up resistor
  DDRD |= (1 << buzzer); //set to output

}

void loop() {

  mpu.getMotion6(&ax, &ay, &az, &gx, &gy, &gz);
  data.X = map(ax, -17000, 17000, 0, 255 );
  data.Y = map(ay, -17000, 17000, 0, 255);
  data.Z = map(az, -17000, 17000, 0, 255);

  currentMillis = millis();

  if ((currentMillis - previousMillis2) >= 2000) {
    previousxlast = xlast;
    previousylast = ylast;
    previouszlast = zlast;
    xlast = data.X;
    ylast = data.Y;
    zlast = data.Z;
    previousMillis2 = currentMillis;
  }

  if ((abs(data.X - xlast) > 4 || abs(data.Y - ylast) > 4) && abs(data.Z - zlast) > 3) {   
    activated = activated + 1;
  }

  if (activated >= 3) {
    if (loopCount < 4) {
      if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) {
        previousMillis = currentMillis;
        noTone(buzzer);

      }
      if (((PIND &= (1 << switchPin) >> switchPin) == 1) && currentMillis - previousMillis >= 5000) {
        previousMillis = currentMillis;
        tone(buzzer, 880, 350);
        loopCount = loopCount + 1;

      }
    }
    if (loopCount >= 4 && loopCount < 35) {
      if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) {
        previousMillis = currentMillis;
        noTone(buzzer);

      }
      if (((PIND &= (1 << switchPin) >> switchPin) == 1) && currentMillis - previousMillis >= 500) {
        previousMillis = currentMillis;
        tone(buzzer, 880, 350);
        loopCount = loopCount + 1;

      }
    }
    if (loopCount >= 40) {
      noTone(buzzer);
    }

    if (abs(data.X - previousxlast) < 2 && abs(data.Y - previousylast) < 2 && abs(data.Z - previouszlast) < 2) {
      restartCount = restartCount - 1;
    }
    if (restartCount == 0) {
      loopCount = 0;
      activated = 0;
      restartCount = 4;
    }
  }

}
```

---

<div class="post-metadata">

**Author:** ![TomGeorge](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/tomgeorge/32/215261_2.png) [@TomGeorge](https://forum.arduino.cc/u/TomGeorge)\
**Post date:** [October 4, 2022, 5:11am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/2 "2022-10-04T05:11:14Z")

</div>

Hi, @jimmyjohnsandwitch  
Welcome to the forum.

> [@How to get the best out of this forum](https://forum.arduino.cc/t/how-to-get-the-best-out-of-this-forum/679966):
>
> See also [FAQ - Arduino Forum](https://forum.arduino.cc/faq) for general rules on forum behaviour and etiquette. Hello, Welcome to the Arduino Forum. This guide explains how to get the best out of this forum. Please read and follow the instructions below. Being new here you might think this is having rules for the sake of rules, but that is not the case. If you don’t follow the guidelines all that happens is there is a long exchange of posts while we try to get you to tell us what we need in order to help you, which is fru…

Can you tell us how your code works, that is what is needed from the switch and GY-521 to operate the buzzer and other outputs.  
A list of I/O would help, then we can advise.

Thanks.. Tom.. 😃 👍 ☕ 🇦🇺

---

<div class="post-metadata">

**Author:** ![killzone\_kid](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/killzone_kid/32/1098074_2.png) [@killzone\_kid](https://forum.arduino.cc/u/killzone_kid)\
**Post date:** [October 4, 2022, 5:15am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/3 "2022-10-04T05:15:59Z")

</div>

> [@jimmyjohnsandwitch](#):
>
> I know nesting a bunch of If statements is bad practice.

Please enlighten us why this is bad and how this affects your implementation

---

<div class="post-metadata">

**Author:** ![Coding\_Badly](https://avatars.discourse-cdn.com/v4/letter/c/f14d63/32.png) [@Coding\_Badly](https://forum.arduino.cc/u/Coding_Badly)\
**Post date:** [October 4, 2022, 5:29am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/4 "2022-10-04T05:29:31Z")

</div>

> [@jimmyjohnsandwitch](#):
>
> I know nesting a bunch of If statements is bad practice.

Yeah. That's a minor ... essentially insignificant ... problem compared to this...

> [@jimmyjohnsandwitch](#):
>
> ```arduino
> if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) {
> 
> ```

It's my bedtime. If you haven't spotted the mistake by the time I get up I'll post more.

---

<div class="post-metadata">

**Author:** ![TomGeorge](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/tomgeorge/32/215261_2.png) [@TomGeorge](https://forum.arduino.cc/u/TomGeorge)\
**Post date:** [October 4, 2022, 6:36am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/5 "2022-10-04T06:36:29Z")

</div>

Hi,  
If you are nesting, it might be worth looking at switch... case function.  
You assign each of your condition combinations a value.  
Then use the switch function to select the output response you want.

> **[switch...case - Arduino Reference](https://www.arduino.cc/reference/tr/language/structure/control-structure/switchcase/)**
>
> The Arduino programming language Reference, organized into Functions, Variable and Constant, and Structure keywords.

> **[Arduino switch: Avoid Problems and Learn the Right Way to use it](https://www.best-microcontroller-projects.com/arduino-switch-case.html)**
>
> Arduino switch case : Find out how to write compact code with multiple conditions and how to reduce long lines of 'if-else' into short code. Get it right First Time.

Tom... 😃 👍 ☕ 🇦🇺

---

<div class="post-metadata">

**Author:** ![GolamMostafa](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/golammostafa/32/239011_2.png) [@GolamMostafa](https://forum.arduino.cc/u/GolamMostafa)\
**Post date:** [October 4, 2022, 6:47am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/6 "2022-10-04T06:47:34Z")

</div>

**1.**

> [@jimmyjohnsandwitch](#):
>
> ```arduino
> DDRD &= ~(1 << switchPin); //set to 0 to make it an input
> PORTD |= (1 << switchPin); //set to 1 to turn on the internal pull up resistor
> DDRD |= (1 << buzzer); //set to output
> 
> ```

The above codes could be replaced by:

```arduino
pinMode(switchPin, INPUT_PULLUP);
pinMode(buzzer, OUTPUT);

```

**2.**

> [@jimmyjohnsandwitch](#):
>
> `if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350)`

The above line could be written as:

```arduino
if (!digitalRead(switchPin) && currentMillis - previousMillis >= 350)`

```

---

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 4, 2022, 6:53am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/7 "2022-10-04T06:53:30Z")

</div>

> [@Coding\_Badly](#):
>
> > [@jimmyjohnsandwitch](#):
> >
> > ```arduino
> > if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) {
> > 
> > ```

The only thing I can think of is that

```arduino
(PIND &= (1 << switchPin) >> switchPin) == 0 

```

isn't equivalent to:

```arduino
  PIND = (PIND & (1 << switchPin)) >> switchPin 

```

Is that it?

---

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 4, 2022, 6:59am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/8 "2022-10-04T06:59:02Z")

</div>

Hi Tom 😃 ,  
I added comments throughout the code to explain it and labeled the pins/components  
Its meant mimic the chime sequence of a typical modern car except using gerometer (Accelerometer+gyroscope) in place of CANBUS system

```cpp
#include "Wire.h"
#include "I2Cdev.h"
#include "MPU6050.h"

const uint8_t buzzer = 3; //this is to create the beeping when driving and not buckled
const uint8_t switchPin = 4; // switchpin is seatbelt "buckled in" switch

unsigned long currentMillis;
unsigned long previousMillis = 0;
unsigned long previousMillis2 = 0;
unsigned int loopCount = 0;
unsigned int activated = 0;
uint8_t restartCount = 4;
uint8_t xlast = 0;
uint8_t ylast = 0;
uint8_t zlast = 0;
uint8_t previousxlast = 0;
uint8_t previousylast = 0;
uint8_t previouszlast = 0;

MPU6050 mpu;
int16_t ax, ay, az;
int16_t gx, gy, gz;

struct MyData {
  byte X;
  byte Y;
  byte Z;
};
MyData data;

void setup() {
  Serial.begin(9600);
  Wire.begin();
  mpu.initialize();
  DDRD &= ~(1 << switchPin); //pinMode(switchPin, Input_Pullup) 
  PORTD |= (1 << switchPin); //sets the internal pullup resistor
  DDRD |= (1 << buzzer); //pinMode(buzzer, output)

}

void loop() {

  mpu.getMotion6(&ax, &ay, &az, &gx, &gy, &gz);
  data.X = map(ax, -17000, 17000, 0, 255 );
  data.Y = map(ay, -17000, 17000, 0, 255);
  data.Z = map(az, -17000, 17000, 0, 255); //simplifying ranges

  currentMillis = millis();

  if ((currentMillis - previousMillis2) >= 2000) { //every 2 seconds cycle gerometer values
    xlast = data.X;// creating a log of gerometer values, to measure change between cycles
    ylast = data.Y;
    zlast = data.Z;
    previousxlast = xlast; // a second log of gerometer values. Reset count starts counting everytime there is no change above noise between cycles. This creates an additonal buffer to prevent the reset loop.
    previousylast = ylast;
    previouszlast = zlast;
    previousMillis2 = currentMillis; 
  }

  if ((abs(data.X - xlast) > 4 || abs(data.Y - ylast) > 4) && abs(data.Z - zlast) > 3) { //If significant change in (X or Y axes) and also Y axis is observed, begin activated loop to determine if its just noise or the car is in motion.
    activated = activated + 1;
  }

  if (activated >= 3) { //after 3 loops, the car is determined to be in motion
    if (loopCount < 4) { // the loop count is counting the seatbelt alarm chimes
      if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) { // if(digitalRead(switchPin) == 0 && elapsed time >= noteDuration... 0 = means button is pressed due to internal pull up resistor
        previousMillis = currentMillis;                                                              
        noTone(buzzer); //if seatbelt is connected during warning phase, nothing else happens

      }
      if (((PIND &= (1 << switchPin) >> switchPin) == 1) && currentMillis - previousMillis >= 5000) { //if the seatbelt is not connected, slow reminder chimes every 5 seconds
        previousMillis = currentMillis;
        tone(buzzer, 880, 350);
        loopCount = loopCount + 1;

      }
    }
    if (loopCount >= 4 && loopCount < 40 ) { /after 4 patient chimes, the angry chiming begins until 35 chimes have completed
      if (((PIND &= (1 << switchPin) >> switchPin) == 0) && currentMillis - previousMillis >= 350) { //if seatbelt is attached during the angry chime sequence, nothing else happens
        previousMillis = currentMillis;
        noTone(buzzer);

      }
      if (((PIND &= (1 << switchPin) >> switchPin) == 1) && currentMillis - previousMillis >= 500) { //no seatbelt means angry chiming 
        previousMillis = currentMillis;
        tone(buzzer, 880, 350);
        loopCount = loopCount + 1;

      }
    }
    if (loopCount >= 40) { // after 40 chimes, give up
      noTone(buzzer);
    }

    if (abs(data.X - previousxlast) < 2 && abs(data.Y - previousylast) < 2 && abs(data.Z - previouszlast) < 2) {
      restartCount = restartCount - 1; //if gerometer does not notice any movement, begin a restart cycle
    }
    if (restartCount == 0) { //once restart cycle ==0, reset everything
      loopCount = 0;
      activated = 0;
      restartCount = 4;
    }
  }

}
```

---

<div class="post-metadata">

**Author:** ![Coding\_Badly](https://avatars.discourse-cdn.com/v4/letter/c/f14d63/32.png) [@Coding\_Badly](https://forum.arduino.cc/u/Coding_Badly)\
**Post date:** [October 4, 2022, 5:59pm UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/9 "2022-10-04T17:59:24Z")

</div>

They are equivalent.

Why are you writing to `PIND`?

---

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 4, 2022, 7:52pm UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/10 "2022-10-04T19:52:26Z")

</div>

WOW. What a brain fart. I'm embarrassed now 🤣  
Thanks, Jimmy

---

<div class="post-metadata">

**Author:** ![kellygray](https://avatars.discourse-cdn.com/v4/letter/k/34f0e0/32.png) [@kellygray](https://forum.arduino.cc/u/kellygray)\
**Post date:** [October 4, 2022, 9:09pm UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/11 "2022-10-04T21:09:37Z")

</div>

The &= is an assignment operator. Let's look at how the compiler will interpret that.

First, it will evaluate (1\<\<switchPin)

Then it will take the results of that, and shift it the other way by switchPin bits. The result of this will be a one unless switchpin is greater than the number of bits in an integer. In that case, the result will be zero.

The program will then compare the result of this to zero.

The result of this will be ANDed with the contents of PIND, and the results written back to PIND

Finally, the result of all of that will evaluated as a logical result, and ANDed with the timeout test, and the IF will branch or not, depending on the results.

---

<div class="post-metadata">

**Author:** ![6v6gt](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/6v6gt/32/225488_2.png) [@6v6gt](https://forum.arduino.cc/u/6v6gt)\
**Post date:** [October 5, 2022, 3:42am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/12 "2022-10-05T03:42:20Z")

</div>

I'd probably do it as a finite state machine with maybe the following states

Waiting for Car in motion  
Car confirmed in motion  
In grace period  
In warning 1 (slow chime)  
In warning 2 (angry chime)  
Seat belt OK  
Seat belt not OK accepted

The main structure would be that of a switch statement and significantly flatter than a series of nested if statements.

So, just as an example, if the current state is "Seat belt OK" and the person removes the seat belt the state returns to "Car confirmed in motion ". If the car stops, the state returns to "Waiting for car in motion "

---

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 10, 2022, 1:04am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/13 "2022-10-10T01:04:36Z")

</div>

Thank you for the idea. I love it. I am very limited on memory... how do you think converting into a FSM would affect memory usage? Would you recommend creating the FSM manually with breaks or using the YA\_FSM library?

---

<div class="post-metadata">

**Author:** ![6v6gt](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/6v6gt/32/225488_2.png) [@6v6gt](https://forum.arduino.cc/u/6v6gt)\
**Post date:** [October 10, 2022, 10:11pm UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/14 "2022-10-10T22:11:18Z")

</div>

I used your example (loosely) to test (and improve) my own FSM model and put it in a simulation. About half of it is debug/comment so there is scope to slim it down. You can play with it here if you are interested: [sketch.ino - Wokwi Arduino and ESP32 Simulator](https://wokwi.com/projects/345144975344796243) . It is a bit slower than real time. To get it to detect motion, you have to hammer away a bit on the green button. The slide switch is for the seat belt.

---

<div class="post-metadata">

**Author:** ![jimmyjohnsandwitch](https://avatars.discourse-cdn.com/v4/letter/j/85e7bf/32.png) [@jimmyjohnsandwitch](https://forum.arduino.cc/u/jimmyjohnsandwitch)\
**Post date:** [October 11, 2022, 2:09am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/15 "2022-10-11T02:09:37Z")

</div>

I am in awe of that. I am in awe of you. That is incredible.

I first started learning how to code 2 months ago. Everyday I have been learning theory, practicing example projects, and reading posts from these forums. I was proud of the code I posted. Its the accumulation of weeks of studying all day and took several days to make and debug. My first project.

It took you a couple of minutes to clean it up and halve the memory used.

I feel amazed by you, overjoyed to see the project be improved, and dismayed that the code I spend days working on and thought was "not perfect but pretty damn good" was actually hot garbage.

Thank you a million for your help and also for humbling me and reminding me that I am a complete novice.

Cheers, Jimmy

---

<div class="post-metadata">

**Author:** ![6v6gt](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/6v6gt/32/225488_2.png) [@6v6gt](https://forum.arduino.cc/u/6v6gt)\
**Post date:** [October 11, 2022, 9:01am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/16 "2022-10-11T09:01:12Z")

</div>

That sort of process which you were modelling, in this case the specific example of a car seat belt alarm, is a classic candidate for a finite state machine solution. You identify the states then the conditions for transition from one state to another and any actions to take on those transitions. Usually a good start is to develop a state diagram such as this simple example and annotate it appropriately:

![image](https://europe1.discourse-cdn.com/arduino/original/4X/b/1/f/b1ff8c673d03fc02b9898d80fb5e858c5091b1a6.png)

The code is then, if you use a model similar to the one I posted, more or less a 1:1 representation of the state diagram.

Incidentally, it did take more than a few minutes to code but I was keen to test/improve my own variant a general solution and put it into a simulator. However, the identification of the states from the original description was indeed quick.

In general, for the best learning experience, it is better to code things like that yourself instead of using a standard ready made library. Then you learn how to solve the problem, not how to use a library.

---

<div class="post-metadata">

**Author:** ![system](https://dub1.discourse-cdn.com/arduino/user_avatar/forum.arduino.cc/system/32/1140315_2.png) [@system](https://forum.arduino.cc/u/system)\
**Post date:** [April 9, 2023, 9:01am UTC](https://forum.arduino.cc/t/help-simplifying-nested-code/1038200/17 "2023-04-09T09:01:56Z")

</div>

Closed on April 9, 2023, 9:01am UTC

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.