Amalgamation of the repository and command / query pattern

Question

I am coding a web based solution which consists of N number of tiers, these are:

• UI
• Web API
• Business

The question I have is related to validation, repository and CRUD operations. I am adopting the command / query pattern whereby I have my repositories for data retrieval only (see below) and its query dependencies injected in. I have seen people use repositories for CRUD too, but then I get the impression it is doing too much thus warrants further abstraction.

public interface IAddressRepository
{
    Address GetById(int id);

    IEnumerable<Address> GetByPersonId(int personId);

    IEnumerable<Address> GetByPostalCode(string postalCode);
}

public AddressRepository(IGetAddressByIdQuery getAddressByIdQuery
                       , IGetAddressesByPersonIdQuery getAddressesByPersonIdQuery
                       , IGetAddressesByPostalCodeQuery getAddressesByPostalCodeQuery)
{
    this.getAddressByIdQuery = getAddressByIdQuery;
    this.getAddressesByPersonIdQuery = getAddressesByPersonIdQuery;
    this.getAddressesByPostalCodeQuery = getAddressesByPostalCodeQuery;
}

Should my repository return validation errors? For example, the GetById(int id) method will be checking if the id > 0 then make a DB call or simply return a new instance, likewise with a postal code lookup. However, I also think going by the SOLID design principle everything should be autonomous thus this approach of returning no validation errors is the correct choice. I choose to NOT return null on all my methods simply because checking for null is considered as an anti-pattern.

The encompassing of the CRUD logic. I have seen uses of a Manager and/or Service classes, for example:

public interface IAddressManager
{
    IEnumerable<Error> Delete(Address address);

    IEnumerable<Error> Save(Address address);

    IEnumerable<Error> IsValid(Address address);
}

public AddressManager(IAddressValidator addressValidator, IAddressCommand addressCommand)
{
    this.addressValidator = addressValidator;
    this.addressCommand = addressCommand;
}

Is the general consensus to have a manager class which encompasses these operations but have the dependencies (the commands injected in)? This way, before I call save(), I validate the object before persisting it. I can also control this behavior via TDD.

Query code example:

public interface IGetAddressByIdQuery
{
    Address Execute(int id);
}

public class GetAddressByIdQuery : IGetAddressByIdQuery
{
    private readonly string connectionString;

    public GetAddressByIdQuery(string connectionString)
    {
        this.connectionString = connectionString;
    }

    public Address Execute(int id)
    {
        using (var conn = new SqlConnection(this.connectionString))
        {
            return conn.Get<Address>(new { Id = id });
        }
    }
}

The data access tier is not apparent as at the moment as the business library is fairly small. I am a firm believer in not creating several libraries for the sake of it. Only create them when it is warranted to do so, i.e. a library is getting too big.

What are your thoughts as to whether or not a better approach / pattern exists?

Answer 1

I'll address your separate classes for the queries first. I definitely see where you're coming from with this approach and I can dig it, but it gives too much duplication as it is.

I assume you'll have more entities than just Addresses so using your approach, you would have to create a new interface for each entity you want to get by its ID. If you instead added some generics, you can do all of this with one interface:

interface IGetEntityByIdQuery<T> {
    T execute(int id);
}

Now you can have these implementations:

class GetAddressByIdQuery : IGetEntityByIdQuery<Address> {
    Address execute(int id) { }
}

class GetOrderByIdQuery : IGetEntityByIdQuery<Order> {
    Order execute(int id) { }
}

However even though I first said I can dig it, this will still be very cumbersome to work with. Having to create a new class for the retrieval for each entity for each parameter? That's too much.

I can agree on a setup where you combine them by entity:

interface IGetEntity<T> {
    T GetEntityById(int id);
    T GetEntityByPersonId(int id);
}

Now on to your remark about CRUD:

I have seen people use repositories for CRUD too but then I get the impression it is doing too much thus warrants further abstraction.

The entire idea behind repositories is CRUD. In fact, your GetXByY classes are simply executing the R part in CRUD. Repositories exist to provide communication from your model to your datasource, which is what we understand under CRUD.

Another red flag here should have been your name choice: when you have a class named XManager you have to really overthink your design and be sure that this is the way to go. Often it is an indication something is wrong because in essence, a "manager" class doesn't convey much meaning.

What does this class do? Other names for such encompassing classes are a lot more informative, for example XFactory indicates the class will construct instances of X. A manager.. manages?

Therefore these actions should be added to your repository. This will now keep these related actions (CRUD) together which prevents any confusion with other programmers.

That being said: it might very well be a suitable name for a class, this is definitely not a general rule. I have had to use XManager classes as well before, but this was more because I simply couldn't come up with a better one (and no, it wasn't a repository ;)).

For more reflection upon the subject, I suggest reading this post.

As for your final question on the topic of errors: why are you doing validation logic in your data access layer? That is way too late!

You should have the guarantee that when an object is constructed, it has valid data. The entire idea behind getters and setters (aka: properties) is that the actual data is hidden and that the data can be validated.

When you construct a new object of type Address, it should be validated. This can be done in several places: properties, constructor, the factory that creates Address objects using some service layer, etc. There are many options to do this validation, but once the object is constructed you should have the implicit guarantee that the object contains valid data.

Lastly, for the boundary checks (like id > 0): this is up to your own preference. You could add a service layer in front of your repository that validates the incoming data and if you have several validation requirements then this might be a good solution. If on the other hand you only have this one boundary check then it's a little minor.

But I haven't come across the explicit recommendation to use a service layer in front of your data access layer just for validation yet, so as far as I'm concerned this is up to yourself.