Where should I put commonly used data access code with logic not fitting to Repository when using Service classes on top of Repository/UnitOrWork?

Question

In my earlier question I was asking about implementing repository/unit of work pattern for large applications built with an ORM framework like EF.

One followup problem I cannot come through right now is where to put codes containing business logic, but still lower-level enough to be used commonly in many other part of the application.

For example here is a few such method:

• Getting all users in one or more roles.
• Getting all cities where a user has privileges within an optional region.
• Getting all measure devices of a given device type, within a given region for which the current user has privileges.
• Finding a product by code, checking if it's visible and throwing exception if not found or not visible.

All of these methods use a UnitOfWork for data access or manipulation, and receive several parameters as in their specification. I think everyone could write a lot more example for such common tasks in a large project. My question is where shall I put tese method implementations? I can see the following options currently.

Option 1: Every method goes to its own service class

public class RegionServices {

  // support DI constructor injection
  public RegionServices(IUnitOfWork work) {...}
  ...
  public IEnumerable<City> GetCitiesForUser(User user, Region region = null) { ... }
  ...
}

public class DeviceServices {
  // support DI constructor injection
  public DeviceServices(IUnitOfWork work) {...}
  ...
  public IEnumerable<Device> GetDevicesForUser(User user, DeviceType type, Region region = null) { ... }
  ...
}

What I don't like about it is that if a higher-level application service needs to call for example 3 or these methods, then it needs to instantiate 3 services, and if I use DI then I even have to put all 3 into the constructor, easily resulting quite a bit of code smell.

Option 2: Creating some kind of Facade for such common data access

public class DataAccessHelper {

  // support DI constructor injection
  public DataAccessHelper(IUnitOfWork work) {...}
  ...
  public IEnumerable<City> GetCitiesForUser(User user, Region region = null) { ... }
  public IEnumerable<Device> GetDevicesForUser(User user, DeviceType type, Region region = null) { ... }
  public IEnumerable<User> GetUsersInRoles(params string[] roleIds) { ... }
  ...
}

I don't like it because it feels like violating the SRP, but its usage can be much more comfortable however.

Option 3: Creating extension methods for the Repositories

public static class DataAccessExtensions {
  public static IEnumerable<City> GetCitiesForUser(this IRepository repo, User user, Region region = null) { ... }
}

Here IRepository is an interface with generic methods like Query<T>, Save<T>, etc. I don't like it either because it feels like I want to give business logic to repositories which is not advisable AFAIK. However, it expresses that these methods are common and lower level than service classes, which I like.

Maybe there are other options as well?... Thank you for the help.

Answer 1

If you say that a certain piece of domain logic needs to look at 3 distinct pieces of information in order to make a decision then we will need to provide this information to it.

Further if we say that each of these distinct pieces can be useful to other parts of the domain then each of them will need to be in its own method also. We can debate whether each query needs to be housed in a separate class or not depending on your domain/design.

The point I wanted to make is that there will be a application service which delegates to one or more Finder classes (classes where your queries are housed), these classes house only queries and then accumulate the results and pass it down to a Domain Service as method params.

The domain service acts on on the received parameters executes the logic and returns the result. This way the domain service is easily testable.

psuedo code

App Service
result1 = finder.query1()
result2 = finder.query2()
result3= yetanotherfinder.query();
domainresult = domainservice.calculate(result1,result2,result3);

Answer 2

Repositories belong to the domain, queries do not (http://www.jefclaes.be/2014/01/repositories-where-did-we-go-wrong_26.html).

You could define explicit queries and query handlers and use those outside of your domain.

public class GetUserStatisticsQuery 
{
    public int UserId { get; set; }
}

public class GetUserStatisticsQueryResult
{
    ...
}

public class GetUserStatisticsQueryHandler : 
    IHandleQuery<GetUserStatisticsQuery, GetUserStatisticsQueryResult> 
{
    public GetUserStatisticsQueryResult Handle(GetUserStatisticsQuery query) 
    {
        ... "SELECT * FROM x" ...
    }
}

var result = _queryExecutor.Execute<GetUserStatisticsQueryResult>(
    new GetUserStatisticsQuery(1));

Answer 3

I'm adding my conclusion as an answer, because I quickly realized that this question is quite relative and not exact, heavily depends on personal favours or design trends.

The comments and the answers helped me in seeing more clearly how things like this should basically be implemented, thank you for all of your effort.

Conclusion

A "repository" should be responsible clearly only for data persisting. Because it doesn't hold any domain logic, or type specific logc, I think it can be represented and implemented as an IRepository interface with generic methods like Save<T>, Delete<T>, Query<T>, GetByID<T>, etc. Please refer to my previous question mentioned in the beginning of my original post.

On the other hand, I think (at least now with my current project) that introducing new class/classes for each lower-level domain logic (in the most cases some kind of querying logic) task is a bit over-engineered solution, which is not needed for me. I mean I don't want to introduce classes like GetUsersInRoles or GetDevicesInRegionWithType, etc. I feel I would end up with a lot of classes, and a lot of boilerplate code when refering them.

I decided to implement the 3rd option, adding static query functions as extensions to IRepository. It can be nicely separated in a Queries project folder, and structured in several static classes each named after the underlying domain model on which it defines operations. For example I've implemented user related queries as follows: in Queries folder I've created a UserQueries.cs file, in which I have:

public static class UserQueries {
  public static IEnumerable<User> GetInRoles(this IRepository repository, params string[] roles) 
  {
    ...
  }
}

This way I can easily and comfortable access such methods via extensions on every IRepository, the methods are unit-testable and support DI (as they are callable on any IRepository implementation). This technique fits best for my current needs.

It can be refactored even further to make it even cleaner. I could introduce "ghost" sealed classes like UserQueriesWrapper and use it to structure the calling code and this way not put every kind of such extensions to IRepository. I mean something like this:

// technical class, wraps an IRepository dummily forwarding all members to the wrapped object
public class RepositoryWrapper : IRepository 
{
  internal RepositoryWrapper(IRepository repository) {...}
}

// technical class for holding user related query extensions
public sealed class UserQueriesWrapper : RepositoryWrapper {
  internal UserQueriesWrapper(IRepository repository) : base(repository) {...}  
}

public static class UserQueries {

 public static UserQueriesWrapper Users(this IRepository repository) {
   return new UserQueriesWrapper(repository);
 }

 public static IEnumerable<User> GetInRoles(this UserQueriesWrapper repository, params string[] roles) 
 {
  ...
  }
}

...
// now I can use it with a nicer and cleaner syntax
var users = _repo.Users().GetInRoles("a", "b");
...

Thank you for the answers and comments again, and please if there is something I didn't notice or any gotcha with this technique, leave a comment here.