Marcell LippI can’t stop to write about the importance of code quality and code readability. I think this is one of the most important point at coding. On the other hand I have been already working for several companies and I sadly realised, that the code quality is really poor at most of the companies, even at the famous ones.
In this article I collected 10 small points. It is not so difficult to follow them and if you are following them your code will be much more readable. I added some examples in C++, but all these points can be easily adapted to any other object oriented programming language.
It can be that some of you will be against my points. First of all there are some mostly old-fashioned coding styles which are contradicting with my points. On the other hand some of these points can be a bit against performance. Of course there are special cases when not all of these points should be blindly followed. But I think these 10 small points can make your code quality much better. I hope you will like the points.
- Use the right variable/function names The rule in principle is pretty easy, in practise it is not always so easy: you should call everything based on that what it is. So avoid variables like i, j, x, y, value, return\_value etc. Name your variables and functions well. I present it with an example:
- Use constants instead of magic numbers Try to avoid the usage of magic numbers in your code, it is making it unreadable. Let me have an example:
- Use enums instead of magic numbers In the code of inexperienced programmers I meet quite often with similar solutions:
- Give a name to complicated conditions In most code there are a lot of complicated conditions, like:
- Split up your function to smaller ones OK, this point is maybe trivial: if you have a function which is too long and/or is doing multiple things just create smaller functions which are decoupling a part of your function and call them from your function.
- Do one only one thing in your functions/methods This is connecting from the previous point. So for example if you call a function as SaveToFile I’m expecting from the function that it is saving something to a file and not changing anything on the current data. If you still need to change something on the data move that part of code to a new function. That’s really important, that your function need to do exactly one thing and nothing more: the thing what is in its name. So for example in a getter function don’t do complicated calculations. In that case just create a separate calculate and save function.
- Make a clear difference between functions and methods It is a personal opinion, but I’m following this point at coding and I think it makes the code readability really better. In my understanding a function is a subprogram with a return value. And in my view a function should not change the state of the object. So it should not change any object level private/public variable. Just read them, calculate the return value based on them and return. So for example in C++ they can be marked at const. The methods are subprograms without return value. So in C++ technically they are the void functions. The main goal of methods is to change the state of the object. Every change in the status of the object needs to be done by using methods.
- Organise your parameters/variables into structs There are several cases when a function have several (3+) parameters. Some functions have a really long parameter list over 10-15 parameters. When you are calling such functions it is pretty difficult to figure out which added value belongs to which parameter. That’s why I think it is better to organise your parameters in one or more structs, like: Instead of:
- Convert functions to classes if needed Especially in old code there are quite often function with multiple input and output parameters. Most cases I would avoid the usage of output parameters. But especially if there are multiple ones it is really better if you are creating a class, having all the parameters as private variables. Create setter for inputs and getter for outputs.
- Cover your code with unit tests
vector> getThem()
{
vector> vector1;
for (vector x : theList)
{
if (x[0] == 4)
{
vector1.add(x);
}
}
return vector1;
}
Do you understand not what is this function doing now? I’m sure, you have no real idea. Now I changed the names in the function, is it more clear now?
vector> getSelectedCells()
{
vector selectedCells;
for (vector int> cell : gameBoard)
{
if (cell[CELL_VALUES::STATUS_VALUE] == STATUS::SELECTED)
{
selectedCells.push_back(cell);
}
}
return selectedCells;
}
What about now? Now it looks it is collecting the selected cells of a game board. Nothing changed, just the naming. That’s why naming is pretty important.
int t = x * 24 * 3600 * 365;
Where are these numbers coming from, what does it mean? It’s a bit unclean.
int t = x * hours_per_day * seconds_per_hour * days_per_year;
Is it easier to understand now?
And let’s use what we already learnt in the first point:
int seconds = years * hours_per_day * seconds_per_hour * days_per_year;
Reorganise a bit:
int seconds = years * days_per_year * hours_per_day * seconds_per_hour;
Now it’s much more clear what is it, isn’t?
switch (status)
{
case 1:
//do something
break?
case 2:
//do something
break;
….
}
Ok, but what is the meaning of status = 2? Who knows. Use rather an enum for status like:
Enum class Status { InProgress, NowAnswer, Error, Ok };
It will make your code much more readable.
If (line > 5 && line < 100 && feature_enabled && !error_found && value == 5)
In such case it is really complicated to figure out what is the condition about. It is a good idea to name the condition and/or the sub conditions. For that you can either use lambda functions or booleans, like:
bool isLineInRange = line > 5 && line < 100;
bool featureActived = feature_enabled && !error_found;
if (isLineInRange && featureActivated && value == 5)
Now it is much easier the figure out the meaning of the condition.
drawRectange(double x, double y, double width, double height, int color_r, int color_b, int color_g)
Rather use:
struct Position
{
Position(double x, double y) :
x(x),
y(y)
{};
double x;
double y;
}
struct Size
{
Size(double width, double height) :
width(width),
height(height)
{};
double with;
double height;
}
struct Color
{
Color(int red, int green, int blue) :
red(red),
green(green),
blue(blue)
{};
int red;
int green;
int blue;
}
drawRectange(Position position, Size size, Color color);
So that when you are calling it, instead of:
drawRectangle(100.5, 54.3, 34.5, 23.3, 222, 144, 68);
You can use:
drawRectangle(Position(100.5, 54.3), Size(34.5, 23.3), Color(222, 144, 68));
I think in this way it is much more clear what is the meaning of each parameter.
Unit tests are really useful to verify your program and to avoid regressions. But it is even really good to have unit tests if you would like to understand a code. Simple you can see for each function and class what is their expected behaviour.
These were my points for this articles. I hope you like them, feel free to extend them if you have any other good idea.
