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 no 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 organize the code more, change the structure of this, abstract that from there, etc.
In fact, in the case of the solution I bring, I stopped because we ran the risk of you saying “dude, this resembles the original code as much as an egg resembles a chestnut.” And you wouldn’t be entirely wrong.
Important! Throughout this post, we will differentiate between functionality/process and code/implementation. Many 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 pseudo-code. Code is one of the possible implementations to carry out the functionality.
In the refactoring exercise, we will focus on the code structure. The new code maintains identical functionality to the original. The processes performed by the original code are the same as those performed by the refactored one. The virtues or errors it had are still here.
Even the process inconsistencies, which we will comment on at the end. Although, now they might be more evident because the new code makes it clearer. This is one of the reasons to keep your code clean, as it makes process errors easier to see.
On the other hand, it’s also important to consider that the solution has a formative nature. The goal is not to make perfect code, but to present a series of mechanisms, preferably varied, to keep code clean and organized.
Finally, before ending this introduction, I want to thank the author of the original code. It shouldn’t seem like we are going to tear their code apart. No one is born knowing, and we took this project because it is interesting. So, once again, thanks to the author for sharing their work with the community.
So, after this lengthy introduction, we start the refactoring with a light 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 interrupts and 2 for controlling the “Insteon” device
- Most of the program’s weight is in setup() and, especially, in loop().
- The code runs linearly, with no greater order than “one thing after another.”
- Code from different functionalities is mixed, such as temperature sensor, user input, communication, etc., with no 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 sweat blood to figure out what each part does.
Doctor’s diagnosis? Spaghetti code, and the good kind, textbook. It just needs cheese on top. Proposed treatment? Bring out the pruning shears!
Proposed Solution
We are going to start untangling this code little by little. Just like if it were a knot that got tangled in a headphone cable when we put it in our pocket.
This way, we will see and present some mechanisms we can apply to obtain clean, understandable, testable, reusable, and maintainable code.
Constants
The first, very simple and obvious. We take the program’s constants and move them to independent files.
In this case, we define two files. ‘Config.h’ which 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 anyone who comes later just by looking at the name. This way, we eliminate certain “magic numbers” that appeared in the original code.
We are not evaluating whether from a process point of view they are correct, make sense to define them this way, or if it would be better another way. That 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 on which we will run 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 “floating around” the project. We create an enum.h file, and here we put the corresponding enumerations.
Thus, for example, we identify the operations or actions performed by the project, 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 evaluating whether they are correct, or if there are too many, or missing. They are the ones the author uses and we keep them. Although, probably, some might be different if they had done the exercise of thinking 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 considers ON/OFF. We could have used a bool variable, for example “isTurnedOn”. But it’s clearer if we have an enumeration, and we leave the door open for additional future states (not started, waiting, error, etc). Also, it’s useful for the training exercise.
Model (Part I)
Now that we have identified the necessary constants and enumerations, it’s time to redefine the architecture. Or rather, to define it, because the original code didn’t really have a planned structure.
The first thing we are going to do is abstract an object, or “model” (or if you get carried away, domain) that encapsulates the logic of our process. Following the name of the original project, we call it, for example, IotThermostat.
This object represents “the bulk” of our project. Almost all the logic associated with the process will be in it. That is, it comes to replace, especially, setup() and loop(), and is the foundation that will support the rest of the project.
Hierarchically, the model will be above all other elements we are going to create. Therefore, and since we cannot build a house starting with the roof, we will return to it later. For now, it’s enough 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. Thus, the project contains a Led, an encoder, a button, a display, a temperature sensor and (although the original code doesn’t know it) a threshold control.
For each of these elements we are going to create an independent object. Normally I would call them “classes” or “models”. But, for better understanding, here we will call them “components”.
You will see that, by “taking them out” of the loop, the code for each of these objects is much easier to understand than when it is entangled in an endless loop().
Thus, 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.hpp
The goal is to create simple components that are solely responsible for a single functionality. That is, the “button” object only handles the button. If something fails with the button, this should be the first file to look at. Similarly, if something fails but it’s not the button, there is no need to look at this component.
These components should have weak dependency on the project. That is, they “should not know” in which project they are mounted. Therefore they are easily reusable in other projects, as well as being easy to test their operation independently.
Now let’s look at each of these components.
ButtonComponent
Simply, a simple button that detects a press via interrupt, with simple debounce.
Also, it provides the methods HasBeenPressed() and Restart() for… I don’t need to tell you. Advantages of giving representative names to 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 architecture and independence. This component “communicates” with the rest through its methods (in this case HasBeenPressed). No mixing internal variables with the main loop or other objects. Each thing separate and compartmentalized.
EncoderComponent
Cousin of ButtonComponent, detects via rotary encoder interrupt.
#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 reading what EncoderCTR is worth, and even less rewriting it.
“Communication” 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 doesn’t exist in the original code. That’s not the case, it does exist, but its responsibility (its code) is diffused in the main loop().
This component is responsible for applying a double threshold control to a variable. It is highly reusable between projects and, having the code independently, 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 controller’s output is a boolean variable. First, because this controller doesn’t need more, it doesn’t need to be extended in the future. And also, because it suits me perfectly to explain something.
Couldn’t 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 we’ve been talking all the time about how important it is to maintain the architecture hierarchy and delimit 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 refer to the biggest 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 this one as well.
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 Led control
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 might ask, is it necessary to create a component just to turn on a Led? Well, it’s not a bad idea, first for cleanliness and to follow the architecture we are defining.
But, more importantly, what today is turning on a Led, tomorrow could be “blink three times” or turn on an 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 get a bit more ambitious, because it’s perfect to explain the role of an abstract class or interface.
The original project uses an LCD screen connected via I2C. This introduces a very strong hardware dependency for the project.
Ideally, we would like our thermostat “model” (domain, core) to work with any screen. That is, we want to get rid of this dependency.
To do this, in reality, our “Thermostat” doesn’t 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 we create an IDisplay class, which 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 our program uses, an abstract implementation of a display, not a particular display.
To be able to use a display, we create a new object, applying an adapter pattern. This object will be called DisplayComponent, and 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.hpp
UserInputService
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?
