The week dedicated to the refactoring workshop has ended, so now it’s time to see one of the possible solutions to the exercise we had proposed.
And we say “one of the possible solutions” because there is not a single correct solution. On the contrary, there are infinite solutions, and each one will have its virtues and defects.
Furthermore, the refactoring exercise could potentially never end. It will always be possible to further organize the code, change its structure, abstract certain parts, etc.
In fact, in the case of the solution that I bring, I stopped because we were already at risk of someone saying “dude, this looks like the original code as much as an apple looks like a chestnut”. And the truth is that you wouldn’t be wrong.
Important! Throughout this post, we are going to differentiate what is functionality/process from code/implementation. Many people think they are the same, but they are not. Functionality is “how you solve a problem,” and you could do it on a napkin or in pseudocode. The code is one of the possible implementations to carry out the functionality.
In the refactoring exercise, we are going to focus on the structure of the code. The new code maintains the functionality identical to the original. The processes that the original code did are the same as the refactored one. The virtues or errors that it had are still here.
Even the inconsistencies of the process, which we will comment on at the end. Although, now they may be more evident because the new code makes them more visible. It is one of the reasons to keep your code clean, which makes errors in the process more easily visible.
On the other hand, it is also important to consider that the solution is educational. The objective is not to create perfect code, but to present a series of mechanisms, as varied as possible, to keep the code clean and organized.
Finally, before ending this introduction, I want to thank the author of the original code. It may seem like we are going to criticize their code. Nobody is born learned, and we have taken on this project because it is interesting. So, again, thanks to the author for sharing their work with the community.
So, once this long introduction is over, we start with the refactoring with an analysis of the original code.
Analysis of the original code
Let’s take a quick look at the original code.
- Composed of a single file
- At the beginning, we have a bunch of global variables and #defines
- Most of the code is in a very long setup() function and a very long loop() function
- It only has 4 functions. 2 because they are needed to manage the interruptions and 2 for the control of the “Insteon” device
- Most of the weight of the program is in the setup() and, above all, in the loop().
- The code runs linearly, without any kind of order other than “one thing after another.”
- It mixes code from different functionalities, such as temperature sensor, user input, communication, etc, without any separation in the responsibility of each part.
That is, a clear example of code that works but is not good. In fact, it works by chance. And if the author has to modify or reuse the code someday, they will have a hard time understanding what each part does.
Diagnosis doctor? Spaghetti code but also of the good kind. It just needs some cheese on top. Proposed treatment? Get out the pruning shears!
Proposed solution
Let’s start to unravel this code little by little. Just as if it were a knot that got tangled up in the wire of some headphones in a pocket.
In this way, we will see and present some mechanisms that we can apply to obtain clean, understandable, testable, reusable, and maintainable code.
Constants
The first, simple and obvious. We take the constants of the program and put them in separate files.
In this case, we define two files. ‘Config.h’ that contains the constants associated with the process
#pragma once
const char* SSID = "YOUR_SSID";
const char* PASSWORD = "YOUR_PASSWORD";
const char* HOST = "192.168.0.XX";
const float DEFAULT_TEMPERATURE = 19;
const float DEFAULT_THRESHOLD = 1;
const float TURN_ON_THRESHOLD = 2;
const String TURN_OFF_URL = "http://192.168.0.XX:XXX/3?02621FF5870F11FF=I=3";
const String TURN_ON_URL = "http://192.168.0.XX:XXX/3?02621FF5870F11FF=I=3";In this file, you will see that the constants DEFAULT_TEMPERATURE, DEFAULT_THRESHOLD, and TURN_ON_THRESHOLD have appeared.
All these constants are clearly identifiable by whoever comes after just by looking at the name. In this way, we eliminate certain “magic numbers” that appeared in the original code.
We do not evaluate whether from the process point of view they are correct, make sense to define them this way, or would be better off another way. It is not the purpose of the exercise. The author uses them, so we keep them. We are simply giving a name to something that is already used.
On the other hand, we create a file “Pinout.h”, which collects the hardware definitions in which we are going to execute the project.
#pragma once
const uint8_t SWITCH_PIN = 14 ;// interrupt 0 Pin 2 in Arduino is a must due to interrupts
const uint8_t CLOCK_PIN = 13; // interrupt 1 Pin 3 in Arduino is a must due to interrupts
const uint8_t DATA_PIN = 11;
const uint8_t LED_PIN = 16;
const uint8_t DHT_PIN = 12;Enums
The next step is to create the enumerations that are “running” through the project. We create an enum file, and here we put the corresponding enumerations.
For example, we identify the operations or actions that the project performs, which are the following.
enum COMMAND_OPERATION
{
INCREASE,
DECREASE,
TURN_ON,
TURN_OFF,
SET_DEFAULT,
SET_SETPOINT
};Again (and not to be “annoying” I won’t repeat it more in this post) we are not going to assess whether they are correct, or if there are too many or too few. They are the ones that the author uses, and we keep them. Although, probably, some of them would be different if they had thought about this from the beginning.
On the other hand, we also find as an enum the possible result of an action on the system,
enum COMMAND_RESULT
{
EMPTY,
VALID,
INVALID,
NOT_CONNECTED
};Finally, we identify as an enum the state in which the project can be.
enum THERMOSTAT_STATUS
{
OFF,
ON,
};In this case, it only contemplates ON/OFF. We could have used a bool variable, for example “isTurnedOn”. But it is clearer if we have an enumeration, and we leave the door open to future additional states (not started, waiting, error, etc). In addition, it is useful for the training exercise.
Model (part I)
Now that we have identified the necessary constants and enumerations, it is time to redefine the architecture. Or rather, to define it, because the original code did not actually have a structure.
The first thing we are going to do is abstract an object, or “model” (or if you want to go further, domain) that encapsulates the logic of our process. To continue with the name of the original project, we call it, for example, IotThermostat.
This object represents the “core” of our project. It will contain almost all the logic associated with the process. That is to say, it comes to replace, above all, the setup() and the loop(), and is the basis that will sustain the rest of the project.
Hierarchically, the model will be above all the other elements we are going to create. Therefore, and since we cannot build a house starting from the roof, we will return to it later. For now, it is enough for us to know that it will exist, and that it is the ‘core’ of our project.
Components
Next, we are going to identify the elements that intervene in our project, at a lower level. So, the project contains a Led, an encoder, a button, a display, a temperature sensor, and (although the original code does not know it) a threshold control.
For one of these elements we are going to create an independent object. Normally, we would call them “classes” or “models”. But, for better understanding, we are going to call them “components.”
You will see that, upon “removing” them from the loop, the code of each of these objects is much easier to understand than when it is entangled in an endless loop().
So, we create a “components” folder and, inside it, the following files.
components/Button.hpp
components/Encoder.hpp
components/Led.hpp
components/Display_I2C_Lcd.hpp
components/TemperatureSensor_DHT11.hpp
components/ThresholdController.hppThe objective is to create simple components that are responsible for a single functionality. That is, the “button” object is only responsible for managing the button. If something goes wrong with the button, it should be the first file to check. Similarly, if something goes wrong, but it is not the button, there is no need to check this component.
These components should have a weak dependency on the project. That is, they “should not know” which project they are mounted in. Therefore, they are easily reusable in other projects, and it is also easy to test their operation independently.
Now let’s see each of these components.
ButtonComponent
Simply put, a simple button that detects a press through an interruption, with a simple debounce.
In addition, it provides the HasBeenPressed() and Restart() methods for… I don’t need to tell you. Advantages of giving representative names to the methods ;)
#pragma once
class ButtonComponent
{
public:
void Init()
{
pinMode(SWITCH_PIN, INPUT_PULLUP);
attachInterruptArg(digitalPinToInterrupt(SWITCH_PIN), ButtonComponent::GetISR, this, FALLING);
}
bool HasBeenPressed()
{
return pressedStatus;
}
void Restart()
{
pressedStatus = false;
lastMillis = millis();
}
void ISR()
{
unsigned long lastMillis;
if((unsigned long)(millis() - lastMillis) >= DEBOUNCE_MILLIS)
{
lastMillis = millis();
pressedStatus = true;
}
}
private:
volatile bool pressedStatus;
const unsigned long DEBOUNCE_MILLIS = 100;
unsigned long lastMillis;
void static GetISR(void* switchComponent)
{
((ButtonComponent*)switchComponent)->ISR();
}
};Important for the architecture and independence. This component “communicates” with the rest through its methods (in this case HasBeenPressed). No mixing of internal variables with the main loop or with other objects. Each thing is separated and compartmentalized.
EncoderComponent
The cousin of the ButtonComponent, it detects the rotary encoder interruption.
#pragma once
class EncoderComponent
{
public:
void Init()
{
pinMode(CLOCK_PIN, INPUT);
pinMode(DATA_PIN, INPUT);
attachInterruptArg(digitalPinToInterrupt(CLOCK_PIN), EncoderComponent::GetISR, this, FALLING);
}
void SetCounter(int value)
{
counter = value;
}
bool HasChanged()
{
return change != 0;
}
int GetCounter()
{
return counter;
}
void IncreaseCounter()
{
counter++;
}
void DecreaseCounter()
{
counter--;
}
void Restart()
{
change = 0;
}
void ISR()
{
if((unsigned long)(millis() - lastMillis) >= DEBOUNCE_MILLIS)
{
lastMillis = millis();
if(digitalRead(DATA_PIN) == LOW)
{
counter++;
}
else
{
counter--;
}
change++;
}
}
private:
volatile int counter;
volatile int change;
const unsigned long DEBOUNCE_MILLIS = 100;
unsigned long lastMillis;
void static GetISR(void* encoderComponent)
{
((EncoderComponent*)encoderComponent)->ISR();
}
};Again, the encoder is responsible for self-managing its state with its internal variables. No one should read its value or rewrite it.
The “communication” of the component with the rest of the program is done through the component’s methods, marking a barrier between “this is mine, this is yours.”
ThesholdController
This component may seem “new”, or that it does not exist in the original code. It is not so, it does exist, but its responsibility (its code) is blurred in the main loop().
This component is responsible for applying a double threshold control to a variable. It is highly reusable between projects and, as its code is independent, it is really easy to implement/check.
class ThresholdController
{
public:
float SetPoint = DEFAULT_TEMPERATURE;
float Threshold = DEFAULT_THRESHOLD;
void TurnOn()
{
isTurnedOn = true;
}
void TurnOff()
{
isTurnedOn = false;
}
bool IsTurnedOn()
{
return isTurnedOn;
}
bool IsTurnedOff()
{
return !isTurnedOn;
}
bool Update(float value)
{
if(IsTurnedOn() && value >= SetPoint + Threshold)
{
TurnOff();
}
if(IsTurnedOff() && value <= SetPoint - Threshold)
{
TurnOn();
}
}
private:
bool isTurnedOn;
};In this case, the output of the controller is a boolean variable. First, because this controller does not need more, it does not need to be expanded in the future. And also, because it is perfect for explaining a concept.
Can we use our STATUS enum, which already has an ON, OFF? Shh… listen to this… engrave it in your head… are you listening? CATEGORICALLY NO.
Remember that we are talking all the time about the importance of maintaining the architecture hierarchy and delimiting the relationships between objects.
If this component, which by definition is at the “base” of the hierarchy and is designed to be isolated, reusable, and self-contained, referred to a global project enum, we would be inverting the pyramid. The base would be referring to the heaviest object.
That, apart from making it impossible to reuse the component in another project, would imply that if at a higher level I need to modify the enum, I would be modifying it here too.
In any case, the ThresholdController should use its own THRESHOLD_CONTROLLER_STATUS enum, different from THERMOSTAT_STATUS.
LedComponent
On the other hand, we have the control of the Led
class LedComponent
{
public:
uint8_t Led_Pin;
LedComponent(uint8_t led_Pin)
{
Led_Pin = led_Pin;
}
void Init()
{
pinMode(Led_Pin, OUTPUT);
TurnOff();
}
void TurnOn()
{
digitalWrite(Led_Pin, HIGH);
}
void TurnOff()
{
digitalWrite(Led_Pin, LOW);
}
};You may ask, do we need to create a component just to turn on a Led? Well, it doesn’t hurt, first for cleanliness and for following the architecture we are defining.
But, more importantly, what is now turning on a Led, tomorrow could be “blink three times” or turn on a Led strip.
By encapsulating it in an object, if necessary, in the future we could change it without having to modify anything else in the project.
Display
Now let’s look at the temperature display. Here, we are going to go a little higher, because it is perfect for explaining the role of an abstract class or interface.
In the original project, an LCD screen connected by I2C is used. This introduces a very strong dependency on the hardware for the project.
Ideally, we would like our “thermostat” to function with any screen. That is to say, we want to eliminate this dependency.
To do this, in reality, our “Thermostat” does not need a screen. It only needs something that allows it to call a Show() method to display the temperature and the setpoint.
So we create a folder called “bases” and inside it we create a class IDisplay that contains the methods we need.
#pragma once
class IDisplay
{
public:
void virtual Init() = 0;
void virtual Show(float temp, float target) = 0;
};This will be the object that our program uses, an abstract implementation of a display, not a particular display.
To use a display, we create a new object, applying an adapter pattern. This object will be called DisplayComponent, and it implements IDisplay.
#pragma once
#include "../bases/IDisplay.hpp"
class DisplayComponent_I2C_Lcd : public IDisplay
{
public:
DisplayComponent_I2C_Lcd()
{
Lcd = nullptr;
}
DisplayComponent_I2C_Lcd(LiquidCrystal_I2C& lcd)
{
Lcd = &lcd;
}
void Init()
{
Lcd->backlight();
Lcd->clear();
Lcd->print("Room Temp:");
Lcd->setCursor(0, 1);
Lcd->print("Setpoint:");
}
void Show(float temp, float target)
{
Lcd->setCursor(11, 0);
Lcd->print(temp, 0);
Lcd->print(" C ");
Lcd->setCursor(11, 1);
Lcd->print(target);
Lcd->print(" C ");
}
private:
LiquidCrystal_I2C* Lcd;
};Temperature Sensor
Similarly, the original project is designed for a single temperature sensor, a DHT11. But what if we wanted a different type of temperature sensor? Or even something other than a temperature sensor?
Well, we do the same as we did for the display. We create an abstract class, or interface, called ISensor.
#pragma once
class ISensor
{
public:
void virtual Init() = 0;
float virtual Read() = 0;
};A continuación, hacemos una implementación concreta del sensor, en un sensor de temperatura DHT11.
#pragma once
#include "../bases/ITemperatureSensor.hpp"
#include <MeanFilterLib.h>
class TemperatureSensorComponent_DHT11: public ITemperatureSensor
{
public:
TemperatureSensorComponent_DHT11() : meanFilter(10)
{
}
MeanFilter<float> meanFilter;
DHTesp dht;
void Init()
{
dht.setup(DHT_PIN, DHTesp::DHT11);
}
float Read()
{
auto rawTemperature = dht.getTemperature();
auto humidity = dht.getHumidity(); //TODO: Esto no lo usa para nada
auto temperature = meanFilter.AddValue(rawTemperature);
Serial.print("\tAVG:\t");
Serial.println(temperature, 0);
return temperature;
}
};Here, to see some variety, we’re going to employ a different solution than IDisplay. The previous approach took the original screen as a parameter in the constructor. In this case, we’ve opted for a simpler solution, which is for the TemperatureSensorComponent_DTH11 to internally instantiate the DTH11.
Note, at this point, it could be argued that I’ve “overstepped” what a refactoring is. But, firstly, the original code was a mess. Secondly, I have my own library that serves me well, is tested thoroughly, and has been extensively verified.
This serves as an example of a common scenario. When you have a set of functions used repeatedly, and you (or an improvement team within a development team) invest time in creating a library that contains them.
In this case, modifying the program to adapt it to the new libraries is considered refactoring (although it could be debated). You’re not altering its functionality; you’re merely maintaining the part of the code that performs this process within an external library.
Services
We now have our basic objects or components, identified with the different elemental parts involved in the program.
Now, there are other, less straightforward parts that are also identified by specific functionalities. For example, we need to manage user interaction and communication.
We require elements in our program that handle these functionalities, or rather, these functional areas. And each of these objects responsible for a functional area I’ll call “services” (with confidence!).
In general, a significant portion of the processing logic will fall under these services. Therefore, logically, they’ll be less likely to be reusable across programs. They can be reused, but typically with some modifications because the process will be different.
But that doesn’t mean we won’t make an effort to keep the dependencies between objects as clean as possible. So, for instance, to communicate between them, we’ll create an object that we’ll call, for example, “Command,” which models an action we need to perform in our project.
To do this, we’ll create a CommandBase structure. This contains the command’s result and the operation to perform.
#pragma once
#include "../constants/enums.hpp"
struct CommandBase
{
COMMAND_RESULT Result;
COMMAND_OPERATION Operation;
};Now, we create a new folder that we’ll call ‘services’ and create these files
services/UserInputService.hpp
services/ComunicationService.hppUserInputService
This service will handle everything related to user inputs. In the project’s case, it will manage the encoder and the pushbutton.
#pragma once
#include <WiFi.h>
#include "../constants/enums.hpp"
struct UserInputCommand : public CommandBase {};
class UserInputService
{
public:
UserInputCommand HandleUserInput(EncoderComponent encoder, ButtonComponent button)
{
UserInputCommand result;
result.Result = COMMAND_RESULT::EMPTY;
CompoundUserInputCommand(result, HandleUserInput(encoder));
CompoundUserInputCommand(result, HandleUserInput(button));
return result;
}
UserInputCommand HandleUserInput(EncoderComponent encoder)
{
UserInputCommand result;
result.Result = COMMAND_RESULT::EMPTY;
if(encoder.HasChanged())
{
result.Result = COMMAND_RESULT::VALID;
result.Operation = COMMAND_OPERATION::SET_SETPOINT;
}
return result;
}
UserInputCommand HandleUserInput(ButtonComponent button)
{
UserInputCommand result;
result.Result = COMMAND_RESULT::EMPTY;
if(button.HasBeenPressed())
{
result.Result = COMMAND_RESULT::VALID;
result.Operation = COMMAND_OPERATION::SET_DEFAULT;
}
return result;
}
private:
UserInputCommand CompoundUserInputCommand(UserInputCommand base, UserInputCommand candidate)
{
UserInputCommand compounded = base;
if(candidate.Result == COMMAND_RESULT::VALID)
{
compounded = candidate;
}
return compounded;
}
};Let’s examine the module’s “communication” with the rest of the program. Firstly, the service needs an encoder and a pushbutton. We have various ways to “pass” these objects from the main program to the service.
For instance, we could pass them in the constructor (injection), define them as variables, and associate them in the Init(). In this case, we’ll simply pass them as parameters in the “handleXXX” functions.
This way, we take advantage to demonstrate overloading to show that our service could work with just a pushbutton, an encoder, or both.
The service’s communication with the rest of the program occurs through a UserInputCommand, which inherits from CommandBase. Why do we inherit from an object if we’re not adding or modifying anything? To prompt the compiler to alert me if I ever mix apples with oranges.
Lastly, it’s worth mentioning that we have a method called CompoundUserInput (guess what, meaningful names again). We only use this method in this case, but if it became a frequent need (which it will), we should consider making CommandXXX stop being a struct and become a class, with the Compound method becoming a method of the class.
CommunicationService
Similarly, the Communication Service encapsulates all the project logic associated with Web communication. I won’t delve deeply into this because it was one of the weaker parts of the original project.
Just know that all the code related to communication is here as it was. Although it’s probably easily improvable, our aim today isn’t to change this.
#pragma once
#include <WiFi.h>
#include "../constants/enums.hpp"
struct CommunicationCommand : public CommandBase {};
class ComunicationService
{
public:
ComunicationService(int port) : server(port)
{
}
void Start()
{
Serial.println();
Serial.print("Connecting to ");
Serial.println(SSID);
WiFi.begin(SSID, PASSWORD);
while(WiFi.status() != WL_CONNECTED)
{
delay(500);
Serial.print(".");
}
Serial.println("");
Serial.println("WiFi connected");
Serial.println(WiFi.localIP());
StartServer();
}
void StartServer()
{
server.begin();
Serial.println("Server started");
}
CommunicationCommand ProcessRequest(String request)
{
CommunicationCommand result;
result.Result = COMMAND_RESULT::EMPTY;
if(request.indexOf("") != -10)
{
if(request.indexOf("/+") != -1)
{
result.Operation = COMMAND_OPERATION::DECREASE;
}
if(request.indexOf("/-") != -1)
{
result.Operation = COMMAND_OPERATION::DECREASE;
}
if(request.indexOf("/ON") != -1)
{
result.Operation = COMMAND_OPERATION::TURN_ON;
}
if(request.indexOf("/OFF") != -1)
{
result.Operation = COMMAND_OPERATION::TURN_OFF;
}
}
else
{
result.Result = COMMAND_RESULT::INVALID;
}
return result;
}
String response = "HTTP/1.1 200 OK\r\n"
"Content-Type: text/html\r\n\r\n"
"<!DOCTYPE HTML>\r\n<html>\r\n"
"<p>Setpoint Temperature <a href='/+'\"><button>+</button></a> <a href='/-'\"><button>-</button></a></p>"
"<p>Boiler Status <a href='/ON'\"><button>ON</button></a> <a href='/OFF'\"><button>OFF</button></a></p>";
void SendResponse(WiFiClient& client, const float temperature, const float setpoint)
{
client.flush();
client.print(response);
client.println();
client.print("Room Temperature = ");
client.println(temperature);
client.println();
client.print("Setpoint = ");
client.println(setpoint);
delay(1);
}
CommunicationCommand HandleCommunications(const float temperature, const float setpoint)
{
CommunicationCommand result;
WiFiClient client = server.available();
if(!client)
{
result.Result = COMMAND_RESULT::NOT_CONNECTED;
}
// TODO: esta espera bloqueante no tiene sentido
while(!client.available())
{
delay(1);
}
auto req = client.readStringUntil('\r');
client.flush();
auto command = ProcessRequest(req);
if(command.Result == COMMAND_RESULT::INVALID)
{
// TODO: este stop tampoco tiene sentido
client.stop();
}
else
{
result = command;
SendResponse(client, temperature, setpoint);
}
return result;
}
private:
WiFiServer server;
};Regarding aspects that do matter, let’s discuss the service’s “communication” with the rest of the program. To initiate the service, we need the port, which conveniently allows us to demonstrate how to pass it from the constructor.
The service’s communication to the rest of the program is, once again, achieved with a CommunicationCommand, inheriting from CommandBase.
InsteonAPI
Finally, we have the file ‘InsteonAPI,’ which contains the calls to interact with the Insteon device.
#pragma once
#include "constants/config.h"
class InsteonApi
{
public:
static void SetOn()
{
PerformAction(TURN_ON_URL);
}
static void SetOff()
{
PerformAction(TURN_OFF_URL);
}
private:
static void PerformAction(const String& URL)
{
HTTPClient http;
http.begin(URL);
http.addHeader("Content-Type", "text/plain");
int httpCode = http.POST("Message from ESP8266");
String payload = http.getString();
Serial.println(httpCode);
Serial.println(payload);
http.end();
}
};There wasn’t much complexity in the original code regarding this aspect. So, we’ve simply encapsulated the calls as static methods within a class.
Well, it’s somewhat common for API or DB calls to exist in purely static objects. It could be debatable. Personally, I feel like putting it in an object. This way, we could inherit from a base object and potentially interact with more types of manufacturers in the future, etc. However, I don’t think it adds much more than the previous solution to this exercise, so it stays like this. If you want to modify it, I leave it as a suggestion.
Most importantly, it serves as an “excuse” to show another possible solution for encapsulating code, more akin to the “normal functions” in an Arduino sketch. Simply put them as static methods within an object to keep them more organized.
And if you don’t want to create the object, then don’t; simply put them in a file. But don’t mix everything together in one file. A minimum level of organization, for heaven’s sake!
Model (Part II)
Now that we’ve seen all the elements of the solution, let’s return to our “big object,” that is, the model or domain of IotThermostat.
#pragma once
#include "constants/config.h"
#include "constants/pinout.h"
#include "constants/enums.hpp"
#include "bases/CommandBase.hpp"
#include "bases/ITemperatureSensor.hpp"
#include "bases/IDisplay.hpp"
#include "components/Led.hpp"
#include "components/Encoder.hpp"
#include "components/Button.hpp"
#include "components/Display_I2C_Lcd.hpp"
#include "components/TemperatureSensor_DHT11.hpp"
#include "components/ThresholdController.hpp"
#include "services/UserInputService.hpp"
#include "services/ComunicationService.hpp"
#include "InsteonApi.h"
class IotThermostatModel
{
public:
IotThermostatModel() : communicationService(303), led(LED_PIN)
{
}
void Init(IDisplay& idisplay, ITemperatureSensor& itempSensor)
{
display = &idisplay;
temperatureSensor = &itempSensor;
display->Init();
temperatureSensor->Init();
encoder.Init();
button.Init();
communicationService.Start();
}
float GetTemperature()
{
ReadTemperature();
return currentTemperature;
}
void SetSetpoint(float value)
{
controller.SetPoint = value;
encoder.SetCounter(value);
}
float GetSetPoint()
{
return controller.SetPoint;
}
void Run()
{
auto userInputResult = inputService.HandleUserInput(encoder, button);
ProcessCommand(userInputResult);
ReadTemperature();
UpdateControllerStatus();
auto communicationResult = communicationService.HandleCommunications(currentTemperature, GetSetPoint());
ProcessCommand(communicationResult);
display->Show(currentTemperature, GetSetPoint());
}
private:
void ReadTemperature()
{
currentTemperature = temperatureSensor->Read();
}
void UpdateControllerStatus()
{
auto isControllerTurnedOn = controller.Update(currentTemperature);
auto newStatus = isControllerTurnedOn ? THERMOSTAT_STATUS::ON : THERMOSTAT_STATUS::OFF;
if(status != newStatus)
{
PerformStatusChange(newStatus);
status = newStatus;
}
}
void PerformStatusChange(THERMOSTAT_STATUS status)
{
switch(status)
{
case OFF:
InsteonApi::SetOff();
led.TurnOff();
break;
case ON:
InsteonApi::SetOn();
led.TurnOn();
break;
default:
break;
}
}
void ProcessCommand(CommandBase command)
{
if(command.Result == COMMAND_RESULT::VALID);
{
ProcessOperation(command.Operation);
Serial.println(controller.SetPoint);
}
}
void ProcessOperation(COMMAND_OPERATION operation)
{
switch(operation)
{
case SET_DEFAULT:
SetSetpoint(DEFAULT_TEMPERATURE);
break;
case SET_SETPOINT:
SetSetpoint(encoder.GetCounter());
break;
case INCREASE:
encoder.DecreaseCounter();
Serial.println("You clicked -");
break;
case DECREASE:
encoder.IncreaseCounter();
Serial.println("You clicked -");
case TURN_ON:
SetSetpoint(currentTemperature + TURN_ON_THRESHOLD);
Serial.println("You clicked Boiler On");
break;
case TURN_OFF:
SetSetpoint(DEFAULT_TEMPERATURE);
Serial.println("You clicked Boiler Off");
break;
default:
break;
}
}
THERMOSTAT_STATUS status = THERMOSTAT_STATUS::OFF;
EncoderComponent encoder;
ButtonComponent button;
LedComponent led;
IDisplay* display;
ISensor* temperatureSensor;
ThresholdController controller;
UserInputService inputService;
ComunicationService communicationService;
float currentTemperature;
};In summary, we have internal variables that refer to components and services, and configure the “skeleton” of the program. These are initiated through the appropriate methods, within the object’s own Init(…).
The most important method is Run(), which carries out the work of the IotThermostat. That is, receiving user input, web communication, reading the temperature, updating the controller, and taking appropriate actions if there is a change in state.
The operation from the main sketch would be as follows.
#include <WiFi.h>
#include <HTTPClient.h>
#include <Wire.h>
#include <LiquidCrystal_I2C.h>
#include <DHTesp.h>
#include "IotThermostatModel.hpp"
LiquidCrystal_I2C lcd(0, 0, 0);
DisplayComponent_I2C_Lcd display(lcd);
TemperatureSensorComponent_DHT11 dht;
IotThermostatModel iotThermostat;
void setup()
{
Serial.begin(115200);
delay(10);
iotThermostat.Init(display, dht);
}
void loop()
{
iotThermostat.Run();
delay(1000);
}Hasn’t the sketch turned out to be a super small, cute little sketch? As a notable point, it’s worth highlighting that we’re initializing the screen and passing it as an injection into the constructor of the IDisplay wrapper.
Contrastingly, for the temperature sensor, we’ve opted for a different solution (it doesn’t require the object but instead instantiates it internally), so this step is unnecessary.
Final Thoughts
We’ve taken advantage of this refactoring exercise to present several ways and mechanisms to organize and maintain our code cleanly.
Of course, I’m not saying you have to do it this way, nor that the code couldn’t be further improved. We could keep improving it and adding interfaces until it could be in an abstract art museum. We could rethink the Serial.print(…) scattered everywhere, and many other things.
Consider this as a learning exercise where I’ve tried to showcase a bit of variety and make it interesting. At this point, a couple of reflections.
Architecture
We’ve defined a basic layered architecture, where we have a model, services, and components.
In this “mini-architecture,” our components identify with the different elemental parts involved in the program. They are easily testable, reusable, responsible for their own functionality, and independent of the global project (model). They form the “base of the pyramid.”
Additionally, we have other parts of the program identified with specific functionality. For example, we need to manage user interaction and communication. We’ve named these services.
Finally, we have our main object, or “model,” which will use components and services to perform its work. The services, in turn, can also use the components.
Unlike the components, which are neutral concerning the global process, the services have a rather strong correlation with the process. That is, it’s challenging for them to be used in another project because they adhere to a specific design.
If you don’t see it yet, we’re using a pyramid structure, with components at the bottom, services above them, and the model at the top. Essentially, we’re introducing a simple layered architecture, familiar to everyone.
We could improve (and complicate) this structure a lot. But it serves to at least see a structure principle and present basic principles of organized code, such as,
- Divide and conquer
- Single responsibility principle
- (Obsessive!) control of relationships and dependencies between elements
Refactoring
The code has exactly the same functionality as the original. Believe it or not, its behavior and functionality are identical to the original. We’ve just cleaned up the code (a lot).
However, while cleaning up the code, we might stumble upon a surprise or two.
void Run()
{
auto userInputResult = inputService.HandleUserInput(encoder, button);
ProcessCommand(userInputResult);
ReadTemperature();
UpdateControllerStatus();
auto communicationResult = communicationService.HandleCommunications(currentTemperature, GetSetPoint());
ProcessCommand(communicationResult);
display->Show(currentTemperature, GetSetPoint());
}Does that order make sense? Have we considered that the received order, either from input or WiFi, takes priority? What happens if two minutes pass between loop execution? Probably not accounted for. Moreover, in WiFi communication, there’s a blocking wait—does that make much sense? Not really.
With the code organized, these process flaws become much more visible. Furthermore, it’s much easier to improve upon them. For instance, the Run() function could greatly improve simply by changing the order of 4 lines ;).
Agile, KISS
For example, we could consider modifying the code to add more types of sensors or different API types. Or enabling user interaction with the machine through other types of devices, like a joystick, a capacitive screen, etc.
For instance, we could contemplate having the UserInputService possess a vector of Inputs inheriting from a common class, IInput, allowing us to dynamically add according to bla bla bla.
Or we could make it work with different types of devices, not just Insteon ones. This would entail creating a common class, ITurnableDevice, representing a device that has ON() and OFF(), and others inheriting from it, and so on bla bla bla bla.
The thing is, in reality, none of these functionalities have been requested. And according to Agile, if a functionality is not required, we shouldn’t implement it. Generally, it’s advised to abstract object interfaces only when necessary.
However, applying Agile also requires common sense. There are cases where it’s crystal clear, and the code screams for an abstract class. In my opinion, removing the dependency with IDisplay and ISensor was evident. Additionally, it requires little effort and served to explain the concept.
Beyond that, the most important aspect is that now our code is easily expandable and maintainable. If in the future we had to add any of these functionalities, abstract classes, etc., we know exactly where to go to modify it and how to do it. THAT is Agile.
Code Cleanliness
We’ve cleaned up the code, following most of the advice seen in this post /4-tips-for-writing-cleaner-code-in-arduino/ and which are some of the usual good practices in Agile.
We’ve used meaningful variable and function names. We’ve used short functions that handle a single thing. We’ve defined our enums, our constants. The code is understandable when read. The little tips we always see.
You know what’s absent? Not a single comment. And they’re not necessary because the goal is for the code to be understood by reading it. Although in a real project, classes and functions would need documentation. But comments within a function? Aah, no.
You know what else isn’t there? Not a single preprocessor directive. Not a #define, not a #ifdef, nothing. And that’s despite creating a program that works for different hardware. But we haven’t had to insert #ifdef DHT11 … in the middle. We’ve used abstract classes. Minimizing the use of macros.
Efficiency
Someone might say, “Luis, you’ve gone from a code of around 300 lines to one of about 1000. That must have an impact on program efficiency and space. Are you sure?” Let’s take a look.
Código original
//Program size: 888.322 bytes (used 68% of a 1.310.720 byte maximum) (26,15 secs)
//Minimum Memory Usage: 40176 bytes (12% of a 327680 byte maximum)Código refactorizado
//Program size: 887.634 bytes (used 68% of a 1.310.720 byte maximum) (15,41 secs)
//Minimum Memory Usage: 40216 bytes (12% of a 327680 byte maximum)In other words, the “clean” code occupies 688 bytes less memory and (on my computer) compiles in 15 seconds compared to 26. How can that be, oooh? Well, that’s how it is.
Conversely, it consumes 40 bytes more of dynamic memory. Which is a ridiculously small increase in exchange for (for example) being able to work with multiple screens or multiple sensors.
In any case, what I always tell you. Write clean, organized, and maintainable code. Don’t try to do the compiler’s job, it’s much smarter and better at it than we are.
Conclusion
That concludes the refactoring workshop. Thanks to all who participated, and I hope you found it interesting and enjoyable.
Did your solution resemble this? Did it have any common elements? Had you considered any of the issues we’ve seen?