Don't violate clean code rules for enhanced DX

Welcome back to software anti-patterns I've seen a lot and that made me cry even more often. Today's episode is about code that at first sight enhances API usage, but violates patterns behind the scenes.

To be maximal practical in this post I will refer to the .save() method located directly on a model. I think you understand the developer experience improvement aspect of it and chances are, you thought about implementing a similar concept too.

Clean code > DX || CleanCode == DX

You want your API users to be as happy as possible. It's the right step and praiseworthy. But just for the sake of reducing lines of code or non-hidden dependencies, I would not recommend violating the SRP (Single Responsibility Principle) or other clean code rules.

The Problem - Implementing a .save() method directly on the model

1. You have a model that should be persisted in any kind of database.

(For this example, it does not matter where it is stored. It might be one of the following.)

• LocalStorage for frontend apps

• MySQL, and PSQL directly on the backend

• Calling an API endpoint to make the backend persistent it

1. You get innovative and start caring about developer experience

One might think: "How should I store my user? Where is all the user data stored? Looks like on the user object, so why not let it take care of saving?".

// I will let the user store himself
 
class User {
  public username: string;
  public firstName: string;
  public lastName: string;
  
  public constructor(username, /*...*/) { /* assign properties */}
 
  save() {
    // what to do here?
  }
}
 
const user = new User();
 
user.save();

1. In the beginning, this just violates the first principles of the S.O.L.I.D principles

One should not approach this, because it violates the Single responsibility principle.

The user should not know how to save himself.

This will get you in trouble when reusing, testing, refactoring or plain speaking everything that has to do with software design.

Fun fact: the original idea (enhancing the experience when using your library) might be achieved when looking at the save call, but will lead to great issues when trying to change or test the implementation.

1. But there's more to it - think further

Think about what you need to save him:

• An open MySql connection?

• The localStorage singleton?

• An authenticated HTTP Provider?

The most obvious way would be to have a Singleton class for either of them three.

"So just make a Singleton out of the http provider. What's the big deal, Alex?!"

With that being sad... Let me continue with the list of negative sides that will arise sooner or later.

Everything negative about this

Duplicate code

You might have implemented a save method on the User. But there will surely be other models that will work similarly.

With a repository in place, you might save a lot of time and duplicate code by thinking about more general principles, like serialization and deserialization.

Testing

Let's imagine you want to test the most simple layer to test - the domain layer.

The core project is now officially married to the persistence layer. One is now not able to run any tests without dealing with this layer.

Burning bridges with other design patterns

By making something a singleton you're burning bridges with almost every other design pattern.

Other things

• Singletons might not be thread-safe.

• Global shared states pollute functions or modules. Pure functions will return the same result when calling multiple times. When a global state changes the result of a function via a side effect, it becomes unpure.

The - in my opinion - right approach

Just create a repository and inject it in the right places. That way you guarantee independence for all parties which makes testing, maintaining and extending the software much easier.

class UserRepository {
  constructor() {
    this.load();
  }
 
  const users = [];
  save(user: User) {
    this.users.push(user);
    window.localStorage.setItem('users', JSON.stringify(this.users))
  }
  load() {
    const persistedUsers = window.localStorage.getItem('users');
    this.users = JSON.parse(persistedUsers ?? '[]');
  }
}
 
const userRepository = new UserRepository();
const user = new User("alexander.panov");
 
userRepository.save(user);

With this base, we can extend it further - once needed! For example by extracting an interface or something else great.

Conclusion - What's left to say?

Rule #1: Don't make me or your colleagues sad when reviewing such code.

Rule #2: Just don't violate common rules of clean software development just to get faster, or even with the right reasons. It stays a bad move.

And if you've come so far, why didn't you subscribe to my newsletter yet? Are you serious? Thanks for reading 😘