[SOLVED]LinkedList not working properly

Hello,

I am trying to create a program that creates a list according to button presses.
I have Button1 (which should add a "1" to the list) and Button2 (which should add a "2" to the list).
After pressing them in some order, I'm using Button3 to print the whole list to the serial monitor.

For the list I am using a Library called LinkedList by Ivan Seidel, which is the same one we used in school for very very basic Lists.

The problem I am facing is that it doesn't correctly print my button presses to the serial monitor after pressing Button3.

At first, it just spammed lots of 0's to the serial monitor on pressing Button3. I don't know how, but somehow I managed to fix that.
But it still doesn't work correctly. It prints 1's and 2's, but not in the correct order nor the correct amount of numbers.

Here is my code:

#include <LinkedList.h>
int Button1 = 5;
int Button2 = 6;
int Button3 = 7;
int lenght;
int value;
LinkedList<int> myList;

void setup(){
  Serial.begin(9600);
  pinMode(Button1, INPUT); //add
  pinMode(Button2, INPUT); //add
  pinMode(Button3, INPUT); //print
}

void loop() {
  if (digitalRead(Button1)) {
    myList.add(1);
    Serial.println("1 pressed");
    delay(500);
  }
  if (digitalRead(Button2)) {
    myList.add(2);
    Serial.println("2 pressed");
    delay(500);
  }
  if (digitalRead(Button3)) {
    lenght = myList.get(myList.size() - 1);
    Serial.print("{");
    for (int i = 0; i <= lenght; i++) {
      value = myList.get(i);
      Serial.print(value);
      if (i == lenght) {
        Serial.print("}");
      }
      else {
        Serial.print(", ");
      }
    }
    delay(500);
  }
}

Any help is appreciated. Thanks

lenght = myList.get(myList.size() - 1);

with this instruction you don't get the list length, but you get the last element in the list

if you want the list length, use

lenght = myList.size();

for (int i = 0; i <= lenght; i++) {

the last element index is length-1, so this should be

for (int i = 0; i < lenght; i++) {

vlc0617:
the last element index is length-1, so this should be

True, but you should rather do

lenght = myList.size() - 1;

Because '<=' is very easily overlooked when scanning through the code. It's just something you normally don't expect to see in a for loop iterating over something by index.

Thank you so much guys!

LightuC:
True, but you should rather do

lenght = myList.size() - 1;

Because '<=' is very easily overlooked when scanning through the code. It's just something you normally don't expect to see in a for loop iterating over something by index.

The issue I have with this is that "lenght" is actually one less than the length of the list, so it's a misleading name. This means if the list is empty, "lenght" will be -1, which might be confusing. It's much more common and idiomatic to just set "lenght" to the length of the list and then iterate from 0 up to but not including the value of "lenght", as in for (int i = 0; i < lenght; i++). In interval notation, it's [0, lenght).

christop:
The issue I have with this is that "lenght" is actually one less than the length of the list, so it's a misleading name.

It's also misspelled.

christop:
The issue I have with this is that "lenght" is actually one less than the length of the list, so it's a misleading name. This means if the list is empty, "lenght" will be -1, which might be confusing. It's much more common and idiomatic to just set "lenght" to the length of the list and then iterate from 0 up to but not including the value of "lenght", as in for (int i = 0; i < lenght; i++). In interval notation, it's [0, lenght).

A very good answer. This is exactly why every developer should be thinking about their variable names whenever they read them in code. Indeed, length is a misleading name, a better one would be lastIndex. Another solution is to loop over length - 1.

for(int i = 0; i < length - 1; i++)

This keeps the meaning of length, but is also more easily to see.

LightuC:
A very good answer. This is exactly why every developer should be thinking about their variable names whenever they read them in code. Indeed, length is a misleading name, a better one would be lastIndex. Another solution is to loop over length - 1.

for(int i = 0; i < length - 1; i++)

This keeps the meaning of length, but is also more easily to see.

Ah, but then you'll miss the last element with that code!

Woah, I didn't expect this many answers. Thank you!

I actually misspelled "length" on purpose. When I first typed it normally, it changed color signaling there is some function to it. I wasn't sure if that would cause an error when using it as an integer.

I understand and agree that length is not the right way to call it.

If my list had 6 entries, it would go from point 0 to 5, but the length would be 6.

So I can just use

for(int i = 0; i <= length; i++)

when I subtract 1 of length before, right?

Thanks again for all the help

christop:
Ah, but then you'll miss the last element with that code!

Indeed, what a stupid mistake ... my mind must've gone completely bananas.

SpaceCows:
So I can just use

for(int i = 0; i <= length; i++)

when I subtract 1 of length before, right?

Thanks again for all the help

Yes, but I would advise to not subtract 1 and instead change your condition from i <= length to i < length, like vlc and christop suggested.