How to keep clean code

Hey,
I want to start a new project and this time i want to keep my code clean and simple to read.

What are your main principles in keeping code clean? i know that it is useful that one function only has to do one thing, but then i create a function for every small step/ calculation i end up with many small functions. then my main loop looks simple but i have a bunch of function.

do you copy these function in another cpp project file which is linked to the main file or do you keep all the functions in your code?

1 Like

The first thing to do is use multiple tabs in the IDE, use one tab for a group of related things, for example all the code used to update a display might be in one tab.

As to judging what exactly goes into one function, well, I'm still trying to answer that question after years of writing C/C++. It's easy to say 'put one thing in each function', but that hides the problem of what counts as 'one thing'. To pick up on your comment about calculations, if they all share something in common then they probably should be in one function with one (or more) of the parameters passed to the function controlling exactly what gets calculated.

There is no right answer to this, you have to play around until you're happy.

thank.
yeah i thought about e.g. a function where a switch case statement is in. so i could do the switch case as one function and all code executed in one case of the switch case into another function. thanks for the tab thing in the arduino ide. will these be saved in a project wile or do i have to open them everytime i open my project?

They will be saved with the project and all opened together. Try it!

For you, as a beginner (I assume), you can use ino files, the same as the single tab you've been using. These will be compiled as one big file as far as the compiler is concerned, but will be separated for you.

You can also use cpp and h files, but I think that's a lesson for another day.

You can bundle code together in a class too, but again I think that's a lesson for another day.

Break your idea/code up on functional blocks.

E.g. you have code to handle user input (serial, buttons etc), you have code to control outputs and maybe display data and you have logic functionality.

Your serial reading becomes one function, button reading becomes a function, output control becomes a function, display functionality becomes a function and logic becomes a function.

Each function can be split into more functions. In the old days a rule of thumb was that a function should fit on a sheet of paper.

You can put functional related functions in a dedicated ino file or .h/.cpp combination.

  • Always start your projects by making a proposed schematic.
    example:

  • As mentioned, tabs are an important tool to keep code organized, however, they make it more difficult to share code with us.

  • I start with a pre formatted skeleton sketch.


//================================================^================================================
//
//  URL
//
//  Name
//
//  Version    YY/MM/DD    Comments
//  =======    ========    ========================================================================
//  1.00       24/05/25    Started writing this sketch
//
//
//

//#include <Wire.h>



//================================================
#define LEDon              HIGH   //PIN---[220R]---A[LED]K---GND
#define LEDoff             LOW

#define PRESSED            LOW    //+5V---[Internal 50k]---PIN---[Switch]---GND
#define RELEASED           HIGH

#define CLOSED             LOW    //+5V---[Internal 50k]---PIN---[Switch]---GND
#define OPENED             HIGH


//================================================^================================================
//                          millis() / micros()   B a s e d   T I M E R S
//================================================^================================================
//To keep the sketch tidy, you can put this structure in a different tab in the IDE
//
//These TIMER objects are non-blocking
struct makeTIMER
{
#define MILLIS             1
#define MICROS             1000  //we can use this value to divide into a variable to get milliseconds

#define ENABLED            true
#define DISABLED           false

#define YES                true
#define NO                 false

#define STILLtiming        0
#define EXPIRED            1
#define TIMERdisabled      2

  //these are the bare minimum "members" needed when defining a TIMER
  int                      TimerType;      //what kind of TIMER is this? MILLIS/MICROS
  unsigned long            Time;           //when the TIMER started
  unsigned long            Interval;       //delay time which we are looking for
  bool                     TimerFlag;      //is the TIMER enabled ? ENABLED/DISABLED
  bool                     Restart;        //do we restart this TIMER   ? YES/NO

  //================================================
  //condition returned: STILLtiming (0), EXPIRED (1) or TIMERdisabled (2)
  //function to check the state of our TIMER  ex: if(myTimer.checkTIMER() == EXPIRED);
  byte checkTIMER()
  {
    //========================
    //is this TIMER enabled ?
    if (TimerFlag == ENABLED)
    {
      //========================
      //has this TIMER expired ?
      if (getTime() - Time >= Interval)
      {
        //========================
        //should this TIMER restart again?
        if (Restart == YES)
        {
          //restart this TIMER
          Time = getTime();
        }

        //this TIMER has expired
        return EXPIRED;
      }

      //========================
      else
      {
        //this TIMER has not expired
        return STILLtiming;
      }

    } //END of   if (TimerFlag == ENABLED)

    //========================
    else
    {
      //this TIMER is disabled
      return TIMERdisabled;
    }

  } //END of   checkTime()

  //================================================
  //function to enable and restart this TIMER  ex: myTimer.enableRestartTIMER();
  void enableRestartTIMER()
  {
    TimerFlag = ENABLED;

    //restart this TIMER
    Time = getTime();

  } //END of   enableRestartTIMER()

  //================================================
  //function to disable this TIMER  ex: myTimer.disableTIMER();
  void disableTIMER()
  {
    TimerFlag = DISABLED;

  } //END of    disableTIMER()

  //================================================
  //function to restart this TIMER  ex: myTimer.restartTIMER();
  void restartTIMER()
  {
    Time = getTime();

  } //END of    restartTIMER()

  //================================================
  //function to force this TIMER to expire ex: myTimer.expireTimer();
  void expireTimer()
  {
    //force this TIMER to expire
    Time = getTime() - Interval;

  } //END of   expireTimer()

  //================================================
  //function to set the Interval for this TIMER ex: myTimer.setInterval(100);
  void setInterval(unsigned long value)
  {
    //set the Interval
    Interval = value;

  } //END of   setInterval()

  //================================================
  //function to return the current time
  unsigned long getTime()
  {
    //return the time             i.e. millis() or micros()
    //========================
    if (TimerType == MILLIS)
    {
      return millis();
    }

    //========================
    else
    {
      return micros();
    }

  } //END of   getTime()

}; //END of   struct makeTIMER


//                             D e f i n e   a l l   a r e   T I M E R S
//================================================^================================================
/*example
  //========================
  makeTIMER toggleLED =
  {
     MILLIS/MICROS, 0, 500ul, ENABLED/DISABLED, YES/NO  //.TimerType, .Time, .Interval, .TimerFlag, .Restart
  };

  Function examples:
  toggleLED.checkTIMER();
  toggleLED.enableRestartTIMER();
  toggleLED.disableTIMER();
  toggleLED.expireTimer();
  toggleLED.setInterval(100ul);
*/

//========================
makeTIMER heartbeatTIMER =
{
  MILLIS, 0, 500ul, ENABLED, YES       //.TimerType, .Time, .Interval, .TimerFlag, .Restart
};

//========================
makeTIMER switchesTIMER =
{
  MILLIS, 0, 5ul, ENABLED, YES        //.TimerType, .Time, .Interval, .TimerFlag, .Restart
};

//========================
makeTIMER machineTIMER =
{
  MICROS, 0, 1000ul, ENABLED, YES      //.TimerType, .Time, .Interval, .TimerFlag, .Restart
};

//========================
makeTIMER commonTIMER =
{
  MILLIS, 0, 1000ul, DISABLED, NO      //.TimerType, .Time, .Interval, .TimerFlag, .Restart
};


//                                     S t a t e   M a c h i n e
//================================================^================================================
//the States in our machine (use better names that mean something to you)
enum STATES : byte
{
  STARTUP, STATE1, STATE2, STATE3, STATE4, FINISHED
};
STATES mState = STARTUP;


//                                            G P I O s
//================================================^================================================
//
//structure to define input objects
struct makeInput
{
  const byte pin;                //the digital input pin number
  unsigned long switchTime;      //the time the switch was closed
  byte lastState;                //the state the input was last in
  byte counter;                  //a counter used to valididate a switch change in state
}; //END of   struct makeInput

//Digital Inputs
//===================================
//define this input which is connected to a PB switch
makeInput mySwitch =
{
  2, 0, OPENED, 0                        //pin, switchTime, lastState, counter
};

byte filter                      = 10;
//TIMER "switches" runs every 5ms.
//5ms * 10 = 50ms is needed to validate a switch change in state.
//A switch change in state is valid "only after" 10 identical changes is detected.
//used to filter out EMI noise in the system

//OUTPUTS
//===================================
const byte testLED               = 12;
const byte heartbeatLED          = 13;


//VARIABLES
//===================================



//                                           s e t u p ( )
//================================================^================================================
void setup()
{
  Serial.begin(115200);
  //Serial.begin(9600);

  //use INPUT_PULLUP so the pin dose not float which can cause faulty readings
  pinMode(mySwitch.pin, INPUT_PULLUP);  

  pinMode(heartbeatLED, OUTPUT);

  digitalWrite(testLED, LEDoff);
  pinMode(testLED, OUTPUT);

} //END of   setup()


//                                            l o o p ( )
//================================================^================================================
void loop()
{
  //========================================================================  T I M E R  heartbeatLED
  //is it time to toggle the heartbeat LED ?
  if (heartbeatTIMER.checkTIMER() == EXPIRED)
  {
    //toggle the heartbeat LED
    digitalWrite(heartbeatLED, digitalRead(heartbeatLED) == HIGH ? LOW : HIGH);
  }

  //========================================================================  T I M E R  switches
  //is it time to check our switches ?
  if (switchesTIMER.checkTIMER() == EXPIRED)
  {
    checkSwitches();
  }

  //========================================================================  T I M E R  machine
  //is it time to service our State Machine ?
  if (machineTIMER.checkTIMER() == EXPIRED)
  {
    checkMachine();
  }

  //================================================
  //other non blocking code goes here
  //================================================

} //END of   loop()


//                                    c h e c k M a c h i n e ( )
//================================================^================================================
void checkMachine()
{
  //================================================
  //service the current "state"
  switch (mState)
  {
    //========================
    case STARTUP:
      {
        //do startup stuff
      }
      break;

    //========================
    case STATE1:
      {
        //condition returned: STILLtiming, EXPIRED or TIMERdisabled
        if (commonTIMER.checkTIMER() == EXPIRED)
        {
          digitalWrite(testLED, LEDoff);

          //we are finished with this TIMER
          commonTIMER.disableTIMER();

          //next state
          mState = STATE2;
        }
      }
      break;

    //========================
    case STATE2:
      {
        //Do something
      }
      break;

    //========================
    case STATE3:
      {
        //Do something
      }
      break;

    //========================
    case STATE4:
      {
        //Do something
      }
      break;

    //========================
    case FINISHED:
      {
        //Do something
      }
      break;

  } //END of  switch/case

} //END of   checkMachine()


//                                   c h e c k S w i t c h e s ( )
//================================================^================================================
void checkSwitches()
{
  byte pinState;

  //========================================================================  mySwitch.pin
  pinState = digitalRead(mySwitch.pin);

  //===================================
  //has this switch changed state ?
  if (mySwitch.lastState != pinState)
  {
    mySwitch.counter++;

    //is this change in state stable ?
    if (mySwitch.counter >= filter)
    {
      //get ready for the next sequence
      mySwitch.counter = 0;

      //update to this new state
      mySwitch.lastState = pinState;

      //========================
      //did this switch go closed ?
      if (pinState == CLOSED)
      {
        //the time this switch closed
        mySwitch.switchTime = millis();

        digitalWrite(testLED, LEDon);

        //set interval to 2 seconds
        commonTIMER.setInterval(2000ul);
        commonTIMER.enableRestartTIMER();

        //next state
        mState = STATE1;
      }

      //========================
      //did this switch go opened ?
      else if (pinState == OPENED)
      {
        Serial.print("The time the switch was closed = ");
        Serial.println(millis() - mySwitch.switchTime);
      }
    }
  }

  //===================================
  //a valid switch change has not been confirmed
  else
  {
    mySwitch.counter = 0;
  }

  //END of mySwitch.pin

  //========================================================================  Next Switch

} //END of   checkSwitches()


//================================================^================================================

i think its hard to describe a set of rules or guideline. You understand how to do it by seeing clean code.

Keringhan and Plauger wanted to write a book describing how to write good programs. They found it too hard and instead wrote a book showing what was wrong with textbook examples of programs. One of the best books i ever read

I make soup.

Everything is in the one *.ino file.

Functions are small and serve one purpose, any of them can fit on one screen.

When a function is, um, functional, it goes "below the fold" where it can happily be ignored.

In the rare event that a function is found to be a problem, I jump to it it with the search function of the text editor.

At the top, all I see is some libraries, some objects, some constants for pins, maybe some global variables and the setup() and loop() functions. In that order, as that is mostly what and all someone reading in needs to see right away, and how they are accustomed to coming across them as they begin to look into the code.

a7

1 Like
  1. Do not use magic numbers ( digitalWrite(2, HIGH); )
  2. Use descriptive names ( digitalWrite(missileLauncher, HIGH); )
  3. Fully comment each line or function. Leave nothing to mis-interpretation.

I may be wrong, but is @Delta_G the coder who starts with little more than comments about what stuff will ultimately do, and when that is reading plausibly goes and turns each set of those comments into code?

And/or am I the one who missed that part in the previous post?

Anyway, the concept is called pseudocode, more or less that's just writing without regard to details be it variable names or syntax or other pesky things.

In any case, the actual code code by the time one is writing it for realz can almost seem to be writing itself.

a7

  • And, everyone here always starts with a flowchart too.

:lying_face:

2 Likes

Thank you so much for all your input ill try to use as much as you described. What program do you use for easy flow charts?

I did some flowcharts on paper but in a digital style they are easier to modify and easier to read.

I‘ll start in the next days with my project and if you want paste the code here for additional input.

Ireally have to say that i do not understand the whole class thing. I had this topic back in the univeristy but we created something like a class for a car and then did some heritage stuff if this is the right word where we definied e.g. race cars and trucks etc. to underand the concept off classes.

But i dont understand why i should write class with separate cpp and header file when i only use one instance of the class in my code for example. I get it if i want to build a cardealer system where everytime i add a new car to my inventory i create a instance of the car class.

  • It was a bit of tongue in cheek comment.

  • However, I do use it when trying to help someone understand what a piece of code does.
    It helps them conceptualize what is happening, especially for old minds like mine.

See this PDF

FlowCharts Using MS Word.pdf (358.8 KB)




At times being a little messy lets you learn the unexpected you would otherwise not. Clean it up later, write ver 2.0 while you're at it.

A clean desk is a sure sign of a sick mind.

If you don't make mistakes, you're slacking in your efforts.
Clean up and document later. Plan well from the start.

Write next versions, they always start clean! When you run out of new twists you will have a clean cut masterpiece.

In war, the first casualty is usually the battle plan.

  • Well it could also mean you put things where they belong, however, now you don't remember where that is.
    :woozy_face:
1 Like

I see where it all is. It's the drawer organization that hides things and that's why they're up where i can see them!

No, really I leave tools in place until I'm done, which may take weeks and then it's time for right things in right places so I can find them later!

you might looks at Program Development in The Unix Programming Environment for a description of a small scripting language to see how it is organized for developement and testing

again thank you all for your valuable input.

i‘ll try to keep as much practises as you mentioned in mind.

in my first project i just startet and put more and more code into the existing code and created stuff to circumvent errors :D. i just added more and more functions ^^.

for the next one i started will small sets of function that control one behaviour, then cleaned the code and stored it separately and went on to the next part. then i thought about putting everything together.

i‘ll definitely now start with a flow chart diagram and exclude all functions for a bigger function in separate ide tabs.

then i can start thinking about putting it into a class as mentioned to reserve namespaces and to keep e.g. global variables locked for only the function where they will be used.

i created a class for my chained shiftregisteres that consist of a bit array which stores if the wired leds are switched on or off. i created some set functions for single leds and led segments and for refreshing the output of the shiftregisters after they have been altered.

i recognised that the class code took up quiet a bit more storage space as the plain code i used before is this normal?

and when i create a bunch of small functions for my code later will it slow down the code because the cpu has always to storage and read the stack and jump to the next code? or will it be a better practise to inline small functions as i have seen in some libraries?
or will the compiler decide on it‘s own if it makes more sense to inline a function with only one or two lines of code?

You are very probably a long way off from needing to worry about this, or pitting functions in line and stuff like that.

Cross those bridges if you ever come to them. In my Arduino projects, I haven't, so far.

a7

That's common, a kind of Rite of Passage. It is how to make Code Spaghetti!

Then you realize and find better ways!

Another path to spaghetti code is writing something then making fundamental changes that the boss or customer demands. You don't want to abandon the work put in (lost cost fallacy) so you edit and weave new features in, maiking a huge mess! That was very common in the 70's and 80's!

You see and want better, a sure sign of real intelligence!
One way is to make the code as flexible as you can so that adding new abilities won't break what rigidly fit an initial purpose.
No matter what you do, someone wants more or else --- if that isn't on a Murphy's Law list, I am surprised!

1 Like