Array Comparison Question: Works but why/how?

Hello, So i have set out on my first off-tutorial project and it is simple but successful. I made a light bar that lights up according to how much light it receives. I used an array on my pins for the LEDs and while comparing ran into some problems.

I figured out how to make it work correctly but i don’t understand why the first and last comparisons use index and it works but when I tried using index on all of them to set LEDs to LOW it didn’t function as intended.

also is there any better way i could have implemented the code to compare values and get my intended results?

I have included a copy of my code for the project.

Thanks for your help!

Icy

Light_bar_experiment.ino (1.04 KB)

My untested rewrite of your code below.

  1. You only need to initialise the serial comms once, not every time in the loop
  2. It makes it easier if you define a constant for the size of the array instead of the magic number 5
  3. There is no point having an array if you then use explicit array indices rather than the loop index. I think what I have changed it to will work as you had.
  4. In your previous code, you cannot use multiple indexes for the array subscript unless it is declared as multi dimensional. Not sure how it even compiled and it was just good fortune that it worked at all.
  5. I assume the ldrPin was on A0 - better to use the ‘A’ to highlight for the readers of your code it is probably an analog value.
  6. minLight and maxLight were not used?
  7. Use the declared types uint8_t, uint16_t, uint32_t, int8_t, etc rather than int to save on storage and make it really clear what the size of your variables are. Especially with array, memory gets used up really quickly and you generally don’t have much of it to start with.
  8. Arrays are zero based, so it is always more natural to scale from 0 to size-1 rather than starting from 1.
  9. Use the map() function to map the values - more obvious what is going on. The effect is the same but the intent becomes clear.
  10. The digitalWrite() uses the ternary operator ?:. You can substitute this for an if/then statement. Look up how to use the operator as it is really useful.
const uint8_t ledPins[] = {12, 11, 10, 9, 8};
const uint8_t NUM_LEDS = sizeof(ledPins)/sizeof(ledPins[0]);
const uint8_t ldrPin = A0;

void setup(){
  Serial.begin(9600);
  for(uint8_t index = 0; index < NUM_LEDS; index++){
	pinMode(ledPins[index], OUTPUT);
	digitalWrite(ledPins[index], LOW);
  }	
  pinMode(ldrPin, INPUT);
}

void loop(){
  //get light amount to int between 1 and 5
  uint8_t light = map(analogRead(ldrPin), 0, 1023, 0, NUM_LEDS-1);

  Serial.println(light);
  for(uint8_t index = 0; index < NUM_LEDS; index++){
   //determine how many lights to turn on
   digitalWrite(ledPins[index], index <= light ? HIGH : LOW);
  }
}

Thanks a lot Marco!

its cool how you managed to cut my code amount in half and still get it to work. I appreciate the amount of information and help you provided, guess I have a lot to learn.

I went through and researched or fixed all of the tips you gave me. I found the ternary operator astounding, its a compacted if statement on one line, that will definitely come in handy!

I did have a question however about your first tip about initializing the Serial comms only once and not every loop. I just can't seem to find or understand what you mean. Thanks again for all of your help!

+1

Icy

marco_c:
4. In your previous code, you cannot use multiple indexes for the array subscript unless it is declared as multi dimensional. Not sure how it even compiled and it was just good fortune that it worked at all.

If you put multiple expressions, separated by the comma operator, where one expression is expected it is calls a “comma expression”. All of the expressions get evaluated but only the leftmost is used: Comma operator - Wikipedia
Note: Since HIGH==1==true and LOW==0==false then:

   digitalWrite(ledPins[index], index <= light ? HIGH : LOW);

acts exactly the same as:

   digitalWrite(ledPins[index], index <= light);

@Icy

I did have a question however about your first tip about initializing the Serial comms only once and not every loop. I just can’t seem to find or understand what you mean

Your original code in setup() had serial.begin() inside the loop. You only need to do it once outside the loop, and I like to do it near the start of setup() to ensure that any messages can get in/out asap.

@johnwasser
Thanks for the explanation. Nice to learn something today. I struggle to see a practical application for this operator, though, without making code more complex to read.

As far as LOW=0=false you are correct. However, the writer of the digitalWrite() function explicitly expects LOW/HIGH and they are entitled to change these definition without notice (although they would be unpopular). So assuming that it will always be 0/1 can produce maintainability problems. LOW/HIGH is a safer option as it will be supported no matter what. Nit picking maybe, but when you write code to be portable to many operating systems, assumptions around magic numbers you are not in control of are likely to become problems that are hard to fix. My personal preference would be to use the C++ boolean type and true/false as the parameter, as I do in many of my own libraries, as this makes it completely independent of any library definitions.

marco_c:
As far as LOW=0=false you are correct. However, the writer of the digitalWrite() function explicitly expects LOW/HIGH and they are entitled to change these definition without notice (although they would be unpopular). So assuming that it will always be 0/1 can produce maintainability problems. LOW/HIGH is a safer option as it will be supported no matter what. Nit picking maybe, but when you write code to be portable to many operating systems, assumptions around magic numbers you are not in control of are likely to problems that are hard to fix. My personal preference would be to use the C++ boolean type and true/false as the parameter, as I do in many of my own libraries, as this makes it completely independent of any library definitions.

Using the boolean type instead of for example byte, lets the compiler, in principle, to perform space optimizations, putting eight booleans inside a single byte. Don't know if the compiler would actually do so, since it incurs processing overhead. Just a thought though.