How to clean up my code

Projects General Guidance
1

Hi. My code seems to be working fine but it is so long and ugly. I have used "for loops" before for analogWrite and was wondering how I might apply that to my code or there might be another way to simplify it.

Here is my code (it's a function) that I would like simplified:

void circleLoop () {
  
  FastLED.setBrightness(BRIGHTNESS );
  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Black;
  leds2[1] = CRGB::Black;
  FastLED.show();

  leds[2] = CRGB::Black;
  leds2[2] = CRGB::Black;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;

  FastLED.show();
  delay (delayTime);

  //////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Red;
  leds2[1] = CRGB::Red;
  FastLED.show();

  leds[2] = CRGB::Black;
  leds2[2] = CRGB::Black;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);

  ////////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Red;
  leds2[1] = CRGB::Red;
  FastLED.show();

  leds[2] = CRGB::Red;
  leds2[2] = CRGB::Red;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);

  /////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Red;
  leds2[1] = CRGB::Red;
  FastLED.show();

  leds[2] = CRGB::Red;
  leds2[2] = CRGB::Red;
  FastLED.show();

  leds[3] = CRGB::Red;
  leds[3] = CRGB::Red;
  FastLED.show();
  delay (delayTime);

  /////////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Red;
  leds2[1] = CRGB::Red;
  FastLED.show();

  leds[2] = CRGB::Red;
  leds2[2] = CRGB::Red;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);

  //////////////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Red;
  leds2[1] = CRGB::Red;
  FastLED.show();

  leds[2] = CRGB::Black;
  leds2[2] = CRGB::Black;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);

  ///////////////////////////////////

  leds[0] = CRGB::Red;
  leds2[0] = CRGB::Red;
  FastLED.show();

  leds[1] = CRGB::Black;
  leds2[1] = CRGB::Black;
  FastLED.show();

  leds[2] = CRGB::Black;
  leds2[2] = CRGB::Black;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);

  /////////////////////////////////////

  leds[0] = CRGB::Black;
  leds2[0] = CRGB::Black;
  FastLED.show();

  leds[1] = CRGB::Black;
  leds2[1] = CRGB::Black;
  FastLED.show();

  leds[2] = CRGB::Black;
  leds2[2] = CRGB::Black;
  FastLED.show();

  leds[3] = CRGB::Black;
  leds2[3] = CRGB::Black;
  FastLED.show();
  delay (delayTime);
}

////////////////////////////////////////////////////
2

Does it do what you want? Yes? Then go on to the next project.

However, this kind of thing which uses lots of delays could be usefully re-written to work without delays. That might be the basis for your next project - allow your Arduino to do something else at the same time as flashing the lights.

Another thing you might consider is storing the sequence in an array and then your function will be very simple - it just executes one step of the stored data instead of having it all written out in the function code.

3

You're using far too many FastLed.show(); only use it when you have set all the colours.

You can use an array to store the patterns and next iterate over the array.

I've used an array of structs. The struct defines the color for the individual leds in the strips and the duration that they must be on. It's probably more flexible to use R, G and B and not e.g. CRGB::Red, but as you used the latter

#define NUM_STRIPS 2
#define NUM_LEDS 4

struct PATTERN
{

  CRGB strip[NUM_STRIPS][NUM_LEDS];
  uint32_t duration;
};

// example pattern
PATTERN patterns[] =
{
  {
    //       strip 1                                             strip 2                                      duration
    {{CRGB::Red, CRGB::Black, CRGB::Black, CRGB::Black}, {CRGB::Red, CRGB::Black, CRGB::Black, CRGB::Black}}, 1000
  },
  {
    {{CRGB::Black, CRGB::Black, CRGB::Black, CRGB::Black}, {CRGB::Black, CRGB::Black, CRGB::Black, CRGB::Black}}, 500
  },
};

Iterating over the patterns

void circleLoop () {

  FastLED.setBrightness(BRIGHTNESS );

  for (uint16_t patCnt = 0; patCnt < sizeof(patterns) / sizeof(patterns[0]); patCnt++)
  {
    // set all the leds
    for (int ledCnt = 0; ledCnt < NUM_LEDS; ledCnt++)
    {
      leds[ledCnt] = patterns[patCnt].strip[0][ledCnt];
      leds2[ledCnt] = patterns[patCnt].strip[1][ledCnt];
    }
    FastLED.show();
    delay(patterns[patCnt].duration);
  }
}

Not tested