Databases, dependency injection and static methods

Question

I'm working on a Next.js app and looking at the best way to structure database access. With Next.js and web based apps in general, there are lots of different "server actions", aka, API calls, there are lots of potential entry points into the app, so what's the best way to structure this. I feel like the usual patterns are all frowned upon these days.

The logical solution in my head is what I guess you might call static dependency injection. So something like this, where there's a static call to initialise the interface with the database. A default is set for normal service, but we can replace that default when testing if we wish. However, this feels far from perfect.

export class User {
  private static db: DbInterface;
  private userData: any;

  constructor(userData: any) {
    this.userData = userData;
  }
  
  public static initialise(db: DbInterface) {
    this.db = db;
  }

  public static loadFromId(id: number): User {
    const userData: any = db.loadUserFromId(id);
    if (userData) return new User(userData);
    return null;
  }
}

User.initialise(new DefaultDbInterface());

So user loading would be

const user = User.loadFromId(1234);
if (!user){
  // handle user not found
}

Another option would be to create a User instance that we pass an instance of the DbInterface to, but that seems a bit of a faff to me, and therefore not perfect either. So something like:

export class User {
  private db: DbInterface;
  private userData: any;

  constructor(db: DbInterface) {
    this.db = db;
  }

  public loadFromId(id: number): User {
    this.userData = db.loadUserFromId(id);
    if (!this.userData) throw new Error("User not found");
    return this;
  }
}

So user loading would be

try {
  const user = new User(new DefaultDbInterface()).loadFromId(1234);
} catch (error) {
  // handle user not found
}

What's are my other options here? What's easiest to work with, maintain and debug?

Answer 1

The first variation using a static method looks similar to the Active Record ORM for Ruby. That library is an implementation of the Active Record Pattern. The Active Record Pattern flagrantly violates Separation of Concerns, although this might not be fatal to your architecture depending on your use case.

I am a little curious about something you said, though:

I feel like the usual patterns are all frowned upon these days.

I'm not sure which patterns you are referring to here, but we typically keep data access and business logic separate. A data access object or repository (often paired with an Object-Relational Mapper) is used to decouple your business classes and entities from data access logic. This is far from an anti-pattern or frowned upon.

Both variations violate Separation of Concerns. To be honest, I wouldn't recommend either one because it puts your database at the center of your dependency chain. The direction of dependencies is the polar opposite of what most applications use these days. We tend to push data storage details to the fringes of our architecture. A database is more typically thought of as infrastructure rather than core, central logic. Business classes should not be dependent on infrastructure details. Instead, flip the dependency around: the database access code should depend on the business class.

Both variations infuse this infrastructure knowledge into entity or business classes. If I had to pick between the two, I would prefer option 1 where the database object is a class level field. This allows User objects to exist in memory independent of a database, so you can isolate the User object for unit testing without needing to mock the database.

Fixing all this awkwardness means adhering to Separation of Concerns. Separate data access logic from business logic. Let the User class encapsulate the business rules for users. Let some data access object be concerned about storage, retrieval, and mapping database rows to and from User objects:

let storage = new UserStorage(); // this is a data access object
let user = storage.find(1234); // returns a fully populated User

let newUser = new User(...);

storage.add(user); // INSERTs the new record in the user table

^{You can pick your own naming convention for data access objects.}

I personally don't like throwing an exception when a record isn't found. Instead, consider returning null. Is a missing record in a table REALLY worth crashing an application? This could be the result of a user data entry mistake, in which case you want to handle this in a friendlier way than returning a 500 Server Error from a web server, or crashing an application. No need to unwind the stack with an error message. Return null, check for null then return the appropriate response to the user.

Exceptions are good for cases where you need to halt execution of a program no matter what in order to prevent data from being corrupted. Corruption can be literal, as in radiation from space flipping bits in RAM, or conceptual (meaning the data would violate a business rule that must always be true). In this case, code querying the database doesn't know whether the non-existence of a record could cause corruption. I think returning null is fine. The caller should determine if a null value constitutes a problem worthy of halting the program.