Manager classes the right way

Some of the advise in this post is specific to Entity Framework, however many points are still general enough for any tech stack.

Let’s talk about architecture, in particular about writing Manager classes (you might have different name for it, like Service). I know that writing Manager classes is considered an anti-patern, especially by those who follow Domain Driven Design. However for the sake of this discussion we will ignore those arguments and assume that we do not care about Domain Driven Design. We will assume that we want to have a collection of manager classes, which will contain all of our business logic (this design pattern is called transaction script).

Transaction script - organizes business logic by procedures where each procedure handles a single request from the presentation.

– Patterns of Enterprise Application Architecture

In our architecture we will have Business Layer (BL) which will be responsible for coordinating communication between our Presentation Layer (PL) and database. BL has one goal - to enforce business rules and make sure that PL can only do what is allowed by business.

PL to BL to DB

This means that PL should have no means of bypassing BL and reaching into database directly. This doesn’t mean that we need to implement some security mechanism to protect our data from hackers, but rather it means that we need to protect ourselves from our own selves. We need to build our architecture in such a way that it makes it impossible to write bad code!

In this post we assume that our BL is just a collection of manager classes that contains all business logic within them. Those manager classes use repositories to communicate to the database. In .NET world our entire Data Access Layer (DAL) is just a DbContext class with a collection DbSets.

BL to DB

Elements of a good Manager class

The way we implement manager classes has paramount effect on the overall quality and success of our application. To write high quality manager classes, we should follow a number of rules.

There should be no communication between manager classes

Manager classes should not know of each other’s existence. This means that each manager should be responsible for a specific problem domain. It means we must be careful in choosing what our manager is for. There might be a few exceptions to this rule (e.g. - PermissionManager needs to be used from within other manager classes).

Manager methods should map to business actions not database operations

Manager operates at a business level, this means methods like AddUser, DeleteContract, UpdateContract have no place in a manager class. Instead those methods should be called RegisterUser, CancelContract and ChangeContractFee. This is not just about naming, this is about forcing ourselves to think about business rules and thus discovering hidden requirements. It is also about writing code that has business meaning and which can later be reused.

Manager methods should only accept parameters of simple types (e.g. - int, string, enum)

Methods that accept objects as parameters should always raise a red flag. Such methods can introduce security issues, bugs, performance issues and ability to bypass business rules. For example consider this method:

public void CancelContract(Contract contract)
{
    contract.CanceledOn = DateTime.UtcNow;
    this.dbContext.SaveChanges();
}

While it looks nice and simple it has some major problems:

It is possible for the caller to change other properties of contract, thus bypassing business rules.
Caller of our method is forced to retrieve contract entity.
If contract is attached to a different DbContext, then changes will not be persisted to database (specific to Entity Framework).

A much better implementation would be this:

public void CancelContract(int contractId)
{
    var contract = this.dbContext.Contracts.Find(contractId);
    contract.CanceledOn = DateTime.UtcNow;
    this.dbContext.SaveChanges();
}

In this case, no matter what the caller does, there is simply no way to mess things up (at least when considering this method in isolation).

Manager methods should never implement cross-cutting concerns

Cross-cutting concerns are things like audit logging and error logging. Cross-cutting concerns are not core business logic, meaning that our application can work just fine without them. Most often cross-cutting concerns should be implemented using domain event listeners. These are the most common candidates for cross-cutting concerns:

Error logging
Audit logging
Email notifications

Manager’s secondary responsibilities

Apart from core business logic manager should also take care of:

Permission checks to ensure that user is allowed to perform a certain business action. (Best to encapsulate that logic in another class like PermissionManager)
State checks to ensure that business action can be taken at this particular time.
Raising domain events.

Naming managers

Naming is extremely important. Having “Manager” suffix in a class name is considered a code smell. Often it is an indicator that the class is doing too much. For example instead of having a ContractManager, we could probably break it down into ContractProcessor, FeeCalculator, PdfPrinter. This isn’t always easy or possible, however one should be wary of “Manager” suffix and keep classes small and focused on one small area of functionality (single responsibility principle).

Manager methods should return `IQueryable<T>` (and not `IList<T>` or `IEnumerable<T>`)

When a manager needs to return a list of entities, it should make use of IQueryable<T>. For example here is a method returning all active users:

public IQueryable<User> GetActiveUsers() {
    return this.dbContext.Users.Where(u => u.IsActive);
}

The benefit of returning IQueryable is that we allow the caller to further define what they want to get, before making a database call. For example the caller can do this:

// Get all records.
var allActiveUsers = userManager.GetActiveUsers().ToList();

// Select only some columns (projection).
var activeUserNames = userManager.GetActiveUsers().Select(u => new { FirstName = u.FirstName, LastName = u.LastName }).ToList();

// Define further filter.
var activeUserWithoutContract = userManager.GetActiveUsers().Where(u => u.ContractId == null).ToList();

This means that we don’t need to pollute our manager with all the different variations of GetActiveUsers. The best part is that the code is still secure. Business rules cannot be bypassed, because there is no way of making any changes to database through IQueryable. It might be possible for a malicious caller to retrieve more data than the manager intended, however in 99% of the cases the caller is you, so this is not a concern.

`IQueryable` extensions

It’s all good if our manager looks like this:

public class UserManager
{
    public IQueryable<User> GetActiveUsers() {
        return this.dbContext.Users.Where(u => u.IsActive);
    }
    
    public IQueryable<User> GetNewUsers() {
        DateTime cutoffDate = DateTime.UtcNow.AddDays(-7);
        return this.dbContext.Users.Where(u => u.CreatedOn > cutoffDate);
    }
    
    public IQueryable<User> GetUsersInAgeGroup(int minAge, int maxAge) {
        DateTime minBirthday = DateTime.UtcNow.AddYears(-maxAge);
        DateTime maxBirthday = DateTime.UtcNow.AddYears(-minAge);
        return this.dbContext.Users.Where(u => u.Birthday >= minBirthday && u.Birthday <= maxBirthday);
    }
    ...
}

However in many cases we might want to reuse those queries from other places (e.g. - other managers). Also what happens if we want to get all active users in a certain age group? Ideally we want to be able to do something like this:

var users = userManager.GetActiveUsers().GetUsersInAgeGroup(20, 35);

No problem! Just implement all these queries as extensions to IQueryable<User>:

public static class Queries
{
    public static IQueryable<User> GetActiveUsers(this IQueryable<User> users) {
        return users.Where(u => u.IsActive);
    }
    
    public static IQueryable<User> GetNewUsers(this IQueryable<User> users) {
        DateTime cutoffDate = DateTime.UtcNow.AddDays(-7);
        return users.Where(u => u.CreatedOn > cutoffDate);
    }
    
    public static IQueryable<User> GetUsersInAgeGroup(this IQueryable<User> users, int minAge, int maxAge) {
        DateTime minBirthday = DateTime.UtcNow.AddYears(-maxAge);
        DateTime maxBirthday = DateTime.UtcNow.AddYears(-minAge);
        return users.Where(u => u.Birthday >= minBirthday && u.Birthday <= maxBirthday);
    }
}

This means our manager just needs to have only 1 single method to return all sorts of data:

public class UserManager
{
    public IQueryable<User> GetUsers() {
        return this.dbContext.Users;
    }
}

This might look problematic for unit testing, however it isn’t. If we mock our DbContext then IQuerable becomes just an in-memory list.

This is all for now and hopefully makes sense. Feedback is very welcome!

22 Bugs