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.

This +1

During development I don't worry too much about housekeeping. I keep the functions I'm working on front and center and move things that work off into other places. There are several concepts to organize by, for C++ programs like these I like to us OOP, but that's certainly not the only way.

Take the balance bot project, the idea was to start with several different sketches. I started by writing code to control the motor all by itself and then moved that into a library class in another file. Then I got code to handle the IMU all by itself and moved that into a library class in another file. Then I worked on the PID and again moved it off into library files. By the time I actually got to putting any actual robot code together, I already had all of those things organized and ready to roll. All they needed in the sketch was a #include. There wasn't a lot to write. It was more like just hooking things together at that point.

When you look at the code there, if you want to see IMU finctions then you go to the IMU file. If you want to see functions related to the WiFi client then you go to that file. If you want to see the overall setup then you look at the main .ino. If all the names make sense and match what the functions actually do then it makes this very easy to read.

I think the biggest mistake that most people make is that they try to write the whole thing at once or they try to add things directly to it. By the time I'm working on any sort of production code, there's already two or three (or sometimes a lot more) other sketches that came before where I built the code I want to add out by itself. Those sketches always look like a mess. I organize them into classes once I have them working.

3 Likes

I also don't worry about users until hardware works. If I've got some piece of hardware or some peripheral I'm trying to get working then the code I'm working with is likely to have pointers and references all over the place and may or may not follow the safety rules around those. I'm not worried that someday someone may forget to initialize that pointer, I make sure it is initialized in MY code and continue getting my device working.

Once I have the hardware figured out and I know how to make things happen, then I usually stop and refactor everything. I'll try to condense things and use safer methods and start to think about making sure it's user-safe code that can be used elsewhere.

I think premature optimization is the sworn enemy. There's nothing worse than tidying everything up into neat functions and modules only to find out that it doesn't work in the first place. Get it working first, then make it pretty.

1 Like

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

Oh yeah. That's me too. Especially when I have to do something that I'm not sure about or something with a bunch of steps.

Here's an example from setting up the CTSU on the R4. I took the flow chart in the user's manual and re-wrote it straight out as comments. Then just filled in the pieces.

void initialCTSUsetup() {
  // Follow the flow chart Fig 41.9
  // Step 1: Discharge LPF (set TSCAP as OUTPUT LOW.)

  // Step 2: Setup I/O port PmnPFR registers

  // Step 3: Enable CTSU in MSTPCRC bit MSTPC3 to 0

  // Step 4: Set CTSU Power Supply (CTSUCR1 register)

  // Step 5: Set CTSU Base Clock (CTSUCR1 and CTSUSO1 registers)

  // Step 6: Power On CTSU (set bits CTSUPON and CTSUCSW in CTSUCR1 at the same time)

  // Step 7: Wait for stabilization (Whatever that means...)
}

Sometimes I'll have whole sketches mapped out like that before I start. I helps me to remember what the original plan was.

2 Likes
  • 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.

In some cases you just take the functions out to another file.

The main reason I write classes out of them is to keep from polluting the name space. I tend to use short readable function names and there might be several different modules in my code that need a read or write or send or something like that. If they're different classes or different namespaces then I don't have to worry if they have the same name for some function.

Sometimes it's hard to make a class so I'll just create a namespace for it instead and just leave them as functions. It usually depends on how much data needs to be shared. I don't like having to put extern declarations in my .ino file so if there are variables that need to be shared or something then a class makes it easier for both files to see everything.

  • 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!