Is This code elegant?

/*
Code written by Dennis Miller.
As a first attempt of coding.
2 74HC595 ICs cascaded to light up one LED at a time till all
are lit up.
Then the MR_Pin is set to low to Zero the register content

And the process starts all over again.

*/
int SHCP_Pin = 8; // latch pin
int STCP_Pin = 9; // Storage Register clock input low to High
int DS_Pin = 10; // Data transferred to Register and output
int MR_Pin = 7; // Memory reset pin

void setup()

{
pinMode(SHCP_Pin, OUTPUT); // Set pins 8,9,10 to output mode
pinMode(STCP_Pin, OUTPUT);
pinMode(DS_Pin, OUTPUT);
pinMode(MR_Pin, OUTPUT);

digitalWrite(SHCP_Pin, LOW);
digitalWrite(STCP_Pin, LOW);
digitalWrite(DS_Pin, LOW);
digitalWrite(MR_Pin, LOW);
}

void loop()

{
for( int i = 0; i <=16; i++)
{ digitalWrite(MR_Pin, HIGH); // set MR_Pin to high
digitalWrite(DS_Pin, HIGH); // set DS_Pin high
delay(10); // introduce a delay
digitalWrite(SHCP_Pin, HIGH); // set SHCP_Pin high to send serial data to the Registers
delay(10); // introduce dealy
digitalWrite(SHCP_Pin, LOW); // set SHCP_Pin low
delay(10); // introduce a delay
digitalWrite(STCP_Pin, HIGH); //set STCP_Pin High to send serial dats from REgisters to output

digitalWrite(DS_Pin, LOW);
delay(10);
digitalWrite(SHCP_Pin, LOW);
delay(10);

digitalWrite(DS_Pin, HIGH);
delay(10);
digitalWrite(SHCP_Pin, HIGH);
delay(10);
digitalWrite(SHCP_Pin, LOW);
delay(10);
digitalWrite(STCP_Pin, HIGH);
delay(10);
digitalWrite(STCP_Pin, LOW);
}
digitalWrite( MR_Pin, HIGH);
delay(10);
digitalWrite(MR_Pin, LOW); // Reset Shiftregister

}

It would be a lot more elegant if you posted it in code tags, much easier to read.

You can Modify the post, select the code, and hit the # above the :wink: :sweat_smile: smilies.

Then the code
will
{
look like
}
this.

Thanks!! Your response much appreciated!!

I'll bear that in mind and attempt to make it more readable!!

Hi Dennis,

If that is your first coding attempt, you've done a good job. Congratulations. I've seen much, much worse code done by people who get paid to do it.

I'd offer two suggestions:

  1. A few more comments before sections of related code.to help others understand it, and you about 1 year from now. e.g.: above every few lines of writes, comment on what that section does.

  2. Whenever you see lines of code repeating themselves, it's usually an opportunity to make code more elegant. e.g.:, the section inside the loop looks like it could be reduced. You could create a simple function, which does the write then adds the delay, which removes all the delays from the main loop, but a 2 line function in this case may not be worth it. Another way to do this is with a for loop and a table, both just inside the main loop. Somehing like (pseudo-ish code):

     table = [
           {MR_Pin, HIGH},            // you can still put comment here to explain certain pins
           {DS_Pin, HIGH}.
           {SHCP_Pin, HIGH},
           {SHCP_Pin, LOW},
           {STCP_Pin, HIGH},
           {DS_Pin, LOW}
           {SHCP_Pin, LOW},
           {DS_Pin, HIGH},
           {SHCP_Pin, HIGH},
           {SHCP_Pin, LOW},
           {STCP_Pin, HIGH},
           {STCP_Pin, LOW}
      ];
      for (int j=0; j < table.length; j++)
      {
            DigitalWrite(table[i].pin, table[i].value);
            delay(10); 
      }

DennisMiller:
Thanks!! Your response much appreciated!!

I'll bear that in mind and attempt to make it more readable!!

Then why don't you go and edit your post, and add the #code tags ?!

// Per.

Hint:

Edit is called 'modify' in this forum software, and can be clicked just above any post of your own.

Many thanks to all who responded . I shall try and implement the tips and suggestions given, and hope that I can write better code.

DennisMiller:
Many thanks to all who responded . I shall try and implement the tips and suggestions given, and hope that I can write better code.

Then go and Modify your first post. Mark up your sketch, press the #-button - click Save and it's fixed.

// Per.