How to write my code simpler?

Hi fellow Arduinoists....

Can someone please help me or just point me in some direction. Im fairly new with arduino but have done some small projects. My problem is that alot of good coders look at my program and say my code is very VERBOSE.

If someone can please show me simplify it, Ill really apreciate it. Ive Googled alot. And the problem is that nobody explains it in plain english. So I would apreciate it if someone can help.

I have attached my code as it exceeds the 9000 characters allowed...

Richelieu_V3.1.3.ino (63.7 KB)

My problem is that alot of good coders look at my program and say my code is very VERBOSE.

Wow you are not kidding.
Once you start labeling variables ending with numbers it is time to start using arrays. Think of an array as a variable variable.
Have a read of this page I wrote about arrays.
http://www.thebox.myzen.co.uk/Tutorial/Arrays.html
For example this:-

unsigned long Accumulator1 = 0;
unsigned long Accumulator2 = 0;
unsigned long Accumulator3 = 0;
unsigned long Accumulator4 = 0;
unsigned long Accumulator5 = 0;
unsigned long Accumulator6 = 0;
unsigned long Accumulator7 = 0;
unsigned long Accumulator8 = 0;
unsigned long Accumulator9 = 0;
unsigned long Accumulator10 = 0;
unsigned long Accumulator11 = 0;
unsigned long Accumulator12 = 0;
unsigned long Accumulator13 = 0;
unsigned long Accumulator14 = 0;
unsigned long Accumulator15 = 0;
unsigned long Accumulator16 = 0;
unsigned long Accumulator17 = 0;
unsigned long Accumulator18 = 0;
unsigned long Accumulator19 = 0;
unsigned long Accumulator20 = 0;
unsigned long Accumulator21 = 0;
unsigned long Accumulator22 = 0;
unsigned long Accumulator23 = 0;
unsigned long Accumulator24 = 0;
unsigned long Accumulator25 = 0;
unsigned long Accumulator26 = 0;
unsigned long Accumulator27 = 0;
unsigned long Accumulator28 = 0;
unsigned long Accumulator29 = 0;
unsigned long Accumulator30 = 0;
unsigned long Accumulator31 = 0;
unsigned long Accumulator32 = 0;
unsigned long Accumulator33 = 0;
unsigned long Accumulator34 = 0;
unsigned long Accumulator35 = 0;
unsigned long Accumulator36 = 0;
unsigned long Accumulator37 = 0;
unsigned long Accumulator38 = 0;
unsigned long Accumulator39 = 0;
unsigned long Accumulator40 = 0;
unsigned long Accumulator41 = 0;
unsigned long Accumulator42 = 0;
unsigned long Accumulator43 = 0;
unsigned long Accumulator44 = 0;
unsigned long Accumulator45 = 0;
unsigned long Accumulator46 = 0;
unsigned long Accumulator47 = 0;
unsigned long Accumulator48 = 0;
unsigned long Accumulator49 = 0;
unsigned long Accumulator50 = 0;
unsigned long Accumulator51 = 0;
unsigned long Accumulator52 = 0;
unsigned long Accumulator53 = 0;
unsigned long Accumulator54 = 0;
unsigned long Accumulator55 = 0;
unsigned long Accumulator56 = 0;
unsigned long Accumulator57 = 0;
unsigned long Accumulator58 = 0;
unsigned long Accumulator59 = 0;
unsigned long Accumulator60 = 0;
unsigned long Accumulator61 = 0;
unsigned long Accumulator62 = 0;
unsigned long Accumulator63 = 0;
unsigned long Accumulator64 = 0;
unsigned long Accumulator65 = 0;
unsigned long Accumulator66 = 0;

can be rewritten as :-

unsigned long Accumulator[66];

Then when it comes to using them you can use a loop and use the loop index variable to address the individual variable.

Sections of code that are similar / identical should be placed in a function.

This is a perfect candidate...

    Wire.beginTransmission(0x20);
    Wire.write(0x12);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x20);
    Wire.write(0x13);
    Wire.write(0b00100000);  // <<< The only value that changes
    Wire.endTransmission();

    Wire.beginTransmission(0x27);
    Wire.write(0x12);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x27);
    Wire.write(0x13);
    Wire.write(0b00000000);
    Wire.endTransmission();

You have an enormous amount of repetition. That is a recipe for typos and it makes updating the code very difficult because you have to make changes in so many different places.

Learn how to use arrays so you don't need long series like

int interval1 = 200;
int interval2 = 400;
int interval3 = 600;
int interval4 = 800;

and have a look at how code is organized into functions in Planning and Implementing a Program. When you find the same (or nearly the same) code in two different places it should go into a function which can then be called whenever needed.

...R

SlevenCalevra:
Can someone please help me or just point me in some direction. Im fairly new with arduino but have done some small projects. My problem is that alot of good coders look at my program and say my code is very VERBOSE.

Does the code work, as in do what you want ?

Hi everyone,

Thanx for the response. I will definitelly look at Arrays and how to use them.
And yes this code works perfectly. Nothing wrongh, No Errors at all.

I have bought the C++ book and Im busy working through it. But it starts veeeerrrrrryyyyy slowly.

And as i have said even that book doesnt explain things very clearly.

But I will definitally have a look at what you guys have posted here and look at all the websites.

"I have bought the C++ book and Im busy working through it. But it starts veeeerrrrrryyyyy slowly."

This site is good too
http://www.cplusplus.com/
This book has been good for looking stuff up

And my book for more electronics related stuff

I tried one of these books, just glossed over reading the first page. Maybe I could have read it when I was in college, not any more tho.

I'm covering some issues noted by others, but these are some of the main issues that jump off the page at me.

  1. Learn about C arrays. Accumulator1 through Accumulator66 need to live in an array. NOTE C numbers arrays from zero, so if you declare:
unsigned long Accumulators[66];

they are numbered from 0 to 65 inclusive, not 1 to 66.

  1. Generate your interval values procedurally. Except for 65 and 66, they are all 200 times the interval number. These can be evaluated at run time, with a couple of special case clauses to deal with 65 and 66.

  2. Learn about C for loops. You've got 60+ copies of this block of code:

  if (currentMillis - Accumulator1 >= interval1) {

  Accumulator1 = currentMillis;

    Wire.beginTransmission(0x20);
    Wire.write(0x12);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x20);
    Wire.write(0x13);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x27);
    Wire.write(0x12);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x27);
    Wire.write(0x13);
    Wire.write(0b00000001);
    Wire.endTransmission();
  }

which hardly differ from one another. Put them in a for loop so you only have one copy, and then use the for loop index to make the necessary modifications. The only things that change are the accumulator in use, the interval, and the four values currently encoded in binary. Those values could be placed in static arrays, and then accessed with the loop index.

  1. Learn about C functions.

I can't be bothered to count them, but you've probably got several hundred copies of these eight lines:

    Wire.beginTransmission(0x20);
    Wire.write(0x12);
    Wire.write(0b00000000);
    Wire.endTransmission();

    Wire.beginTransmission(0x20);
    Wire.write(0x13);
    Wire.write(0b00000000);
    Wire.endTransmission();

which only differ in three respects: the value passed to the two calls to Wire.beginTransmission() and the two values in binary that are the parameters to the second and fourth calls to Wire.write().

Those many copies can be replaced by one copy of a function that does the above, and passes the three variable quantities as parameters.

Do that lot, and you should be able to reduce that from 2700 line to probably no more than a couple of hundred, if that.

  1. Maybe try and take a C / C++ course at a local college. There's an immense amount to the language, learning to be an effective C / C++ programmer will stand you in very good stead if you're planning a career in the embedded space. I know this, that's where I spent the first ten years of my career. :wink: