Introduction
Recently I've had to work on code which seemed to be based on clean architecture, but after a while I concluded that it is probably not and it made me think about clean coding in general and the principles of clean architecture in particular. Is it about a set of rules, and we are guaranteed to achieve cleanness as long as we follow them? Or is it about abstract principles, which we can break sometimes as long as we follow the general idea?
It's probably a bit of both, but before getting into it, it is also important to keep in mind why we might want clean code.
As engineers a big part of our job is reading and understanding code. Very often it's our own, sometimes it's someone else's on a project we are familiar with, and sometimes on a project we never saw before. If we see code as a way to convey information, on top of a way to implement a set of algorithms, then the need for clean code is clear. And it's definition is clear too: code that is easy to read and understand. This makes your colleagues' work easier when they have to read your code: for a review, because you're several engineers on the same project, because you're leaving the company and so on. But as importantly (or more), it makes your job easier: you are your most frequent reader. Even if understanding your code is easier for you than for other people, if your code is dirty you will have from time to time a hard time understanding what you wrote yourself.
Clean architecture is a tool to organize projects so that they are easy to understand and to maintain. A big point of how this is done is the heavy focus on the dependency inversion principle, which can be summarized as "Depend on abstractions, not implementations". Many guidelines seem to say that you should define interfaces for your classes, which is fine as long as we understand two things:
- what is an interface
- what is a dependency
Here I want to have a look at a few issues I have noticed, for which the origin is a misconception about one of these concepts. These issues can make it look like the project was made without really thinking about architecture, and then interfaces were added here and there to make it look like clean architecture.
Just to be clear here, the "issues" I talk about here don't necessarily mean that the project is badly coded or that it needs to be refactored ASAP. It's just that I saw things claiming to be clean architecture, looked like clean architecture, but didn't feel like clean architecture. I thought about why it didn't feel like clean architecture and I'm writing my conclusions here.
This article doesn't really dive deep in the concepts of clean architecture. Knowing about it is probably a plus but it's not actually necessary to understand what I'm talking about in this article. Still, Clean architecture and Clean code are very good reads for any software engineer. I read clean code for the first time 10 years ago and it's still my favorite engineering book.
An interface is not an interface
The interface keyword might actually hide an implementation
When clean architecture is a requirement of the project, it might come as an afterthought: you write some code that meets the specs, and then you make sure it follows the dependency inversion principle. So you add an interface for all of your classes and you replace the class with the interface in all your object declarations. You then have an issue: you were accessing the class's member variables outside of the class, not just functions. No problem, you just add a getter to your interface right?
Wrong.
In many cases, maybe most, class variables heavily depend on your particular implementation. Which mean that if you have a getter for such variables in your interface, you're actually depending on concretions, not abstractions. Consider the following representation of a square:
type Point = { x: number; y: number };
interface Square {
getTopLeft(): Point;
getTopRight(): Point;
getBottomLeft(): Point;
getBottomLeft(): Point;
}
Is it an interface? In typescript/java terminology yes, because we declared it to be. In clear architecture terms? probably not. In fact it could probably be replaced with a type, which is a good indication that it's not actually abstract. The main issue here is that the interface definition doesn't tell us how we can interact with the underlying data. It only tells us that a square has four points and that we can access them. To some extent, it also actually specifies the implementation: the implementation will hold the four corners as cartesian coordinate 2d points. Sure, theoretically it could hold one corner and 2 vectors, in polar coordinates, and compute the cartesian coordinate of each point each time the getters are called, but the interface definition pushes us towards a specific implementation.
Another issue is that the interface doesn't tell us anything about what we can do with the data accessed by the getters. Can we modify it? will it modify the square? or is it a copy? Who knows.
How to define a good interface for our square depends on what we want to do with it. Maybe we want to be able to draw it and apply some geometric transformations, or maybe we want to be able to compute its area and perimeter, or maybe we want to test whether points are inside the square's area or not, and so on. A good interface defines actions which can be performed on our underlying data, without knowing about the data structure. Probably something like this:
interface Square {
draw(canvas: Canvas): void;
translate(x: number, y: number): void;
rotate(pivotX: number, pivotY: number, rotationRadians: number): void;
}
Another nice point of coding like this is that it also makes your code more general. This interface could actually now be called Polygon, because its definition doesn't imply that it needs to have four points. A sidenote here: if you're not actually using one of the functions in your code, deleting it from the interface is a good idea. Unused code is noise, and it makes understanding your project harder. It also makes interfaces too specific.
To sum up a bit, defining an interface is not about typing the word interface. It's about thinking about what are the actions that we want to perform on the underlying data, and how to define these actions while giving as little information as possible on the underlying data.
Having a lot of getters in your "interface" can be a good indication that it is not actually an interface. Of course this is not a hard rule, having some getters in interfaces can be perfectly fine, especially if the getters give access to interfaces themselves.
An abstract class can be an interface
On the other side of the same issue, even if you don't type in the word interface, you might actually be defining one. If we keep in mind that the main goal of interfaces is to define how our environment interact with some data, defining a default implementation for some functions might be completely fine, as long as it doesn't force the data representation too much. If we consider our Square interface, it could be ok to define it as follows:
abstract class Square {
public abstract translate(x: number, y: number): void;
public abstract rotate(
pivotX: number,
pivotY: number,
rotationRadians: number
): void;
public draw(canvas: Canvas): void {
this.drawArea(canvas);
this.drawEdges(canvas);
this.drawPoints(canvas);
}
protected abstract drawArea(canvas: Canvas): void;
protected abstract drawEdges(canvas: Canvas): void;
protected abstract drawPoints(canvas: Canvas): void;
}
We are still free to choose how we represent the square itself and the coordinate system that it uses. The additional functions are protected, so callers of this abstract class are not required to know that it has an area, edges and points. And as a bonus we implemented the template method pattern, and using patterns is a very good way to make your code understandable. For the square interface, it might be overkill, because it will not have many implementations anyways. If you remember that we were able to rename our interface Polygon however, it can allow us to reduce a lot of duplication if it is implemented in TriangleImplementation, SquareImplementation, PentagonImplementation and so on.
I'm not saying you definitely should code like this. It may feel cleaner to create an interface defined with the keyword interface, and inheriting it in an abstract class, and maybe it is (although multiplying source files and inheritance can make code hard to read). What I have seen though, when there is one interface for each class, is that it can lead people to forget that they can also reuse code by extending classes. Clean architecture is not separate from the principles of object oriented programming, and using one should not prevent you to use the other.
Dependencies are not imports
Obviously, imports create dependencies, but dependencies can be hidden, and not actually written down.
Consider the following interface for querying a database:
type Customer = {
firstName: string;
lastName: string;
age: number;
};
type Query = {
table: string;
queryString: string;
args: string[];
};
interface DatabaseHelper {
queryCustomer(query: Query): Customer;
}
This interface doesn't import anything, and you can probably use it for all kinds of sql libraries. It does however create an implicit dependency through the type Query that was created. This type forces a very specific type of queries, which means that if we change the database framework, we will probably need to change this interface too, which completely goes against the whole point of clean architecture and the dependency inversion principle.
The fundamental issue with this kind of interface is that it forces code that calls it to break the single responsibility principle. Any code calling this interface needs to know the database framework used (a flavor of sql probably?), and it needs to create correct queries for that framework. In terms of readability, it also forces the user to manipulate concepts at different levels of abstraction: the database interface (high) and the queries (low).
The interface should probably be replaced with something like this:
type Customer = {
firstName: string,
lastName: string,
age: number,
};
interface DatabaseHelper{
queryCustomerByName(first: string, last: string): Customer;
queryCustomersByAge(age: number): Customer[];
...
}
I'm not sure whether this example was very clear, but my point is: you can create dependencies to a library just by depending on types that you created for that library, and which are actually very specific to it.
There is a simple way to see when your interface has the issue I am talking about here. Look at the signature of its functions, and in particular at the types in it, and ask yourself: "should all code calling this function know about these types". If the answer is no, then it might be a good idea to think about this interface a bit more. You might decide that it's fine as it is, sometimes clean coding principles need to be broken. One of the points of principles is to be aware of them when we break them.
Conclusion
Clean coding and clean architecture are very good tools to make our code easy to understand and to maintain. Guidelines and templates can be very useful in implementing them, however they do not replace thinking about the code. There is no template which will work for all projects, and the best architecture for a project might become bad as the project itself evolves. We need to be aware of the rules, and weigh them against our goals, so that when we break them we know that we break them and why.
We're Hiring!
Are you fluent in Japanese and looking for a new challenge? Head over to Spacely/Recruit and check out our current job openings!
