1

In the repository pattern, is it a "bad idea" to call a service method from another service method?

In most cases, I naturally end up calling repository methods from my service methods. However, in some cases, calling a service method from another service method seems to make sense because the existing service method nicely handles the task at hand.

Let's say I have the following methods:

GetUserFavoritesServiceMethod(userId, requestByUserId)
{
   // Make sure the request is allowed
   // This is the business logic where I make sure the requesting party is authorized to make the request

  // If allowed, call repository method
  var favs = GetUserFavoritesRepositoryMethod();
}

GetUserFavoritesRepositoryMethod(userId)
{
   // Return results
}

Now, let's assume I have another service methods that needs user favorites. I may be overly simplifying my example but this is where I have the option to call either the repository method or the existing service method.

GetSomeOtherInfoServiceMethod(userId, requestByUserId)
{
   // Call GetUserFavoritesServiceMethod() OR GetUserFavoritesRepositoryMethod()
}

Calling the GetUserFavoritesServiceMethods() seems a bit cumbersome because I may already be performing some business logic in the GetSomeOtherInfoServiceMethod() and performing business logic all over again in the GetUserFavoritesServiceMethod() seems bad/wasteful/etc.

On the other hand, the GetUserFavoritesServiceMethod() may be nicely packaging the data for me as opposed to GetUserFavoritesRepositoryMethod() may not be.

Again, I'm over simplifying it but clearly I'm adding some "value" in service methods and sometimes I want to take advantage of that "value".

I want to see if calling a service method from another service method is frowned upon in general.

1

You are facing a problem of an anemic domain model which leads to uncertainty about where to put "general" business logic which is unspecific to a usecase.

There are two types of business logic:

  1. usecase specific
  2. usecase unspecific

Watch this statements from Robert C. Martin:

OOP 2015 Keynote - Robert C. Martin ("Uncle Bob"): Agility and Architecture (26:55 to 28:22)

Robert C. Martin locates the usecase specific business logic into objects called "interactors". You also can say "usecases". Maybe "services" are a closely related so you may go on with that.

Entities will contain business logic that is not usecase specific. He also says that some developers call them "business objects" so I would do.

Whatever you call it: usecase specific and usecase unspecific business logic are different and have to be separated if you ask for advice. So an object of the domain model should be appropriate location to put the code of the method "getUserFavorites".

My suggestion is: user.getFavorites();

  • Excellent points! I guess the trick is to figure out what's use-case specific and what is not. I also suspect we may see that what we deemed use-case unspecific may turn out to be use-case specific. That's what architecture is all about though. Thanks again. – Sam Aug 6 '17 at 20:49
0

It's fine if you're clear about the dependency. What's a service here but a name? You could inject all sorts of dependencies here.

public class AddedInfoFavoritesService
{
    private IFavoritesService favService;

    public AddedInfoFavoritesService(IFavoritesService favService)
    {
        this.favService = favService;
    }

    public FavoriteInfo GetSomeOtherInfoServiceMethod()
    {
        var fav = favService.GetUserFavoritesServiceMethod();
    }
}

I really don't care about the search details here as long as IFavoritesService gives me a collection of user favorites.

The problem is when you hide the service dependency. But that's its own problem in general.

public FavoriteInfo GetSomeOtherInfoServiceMethod()
{
    var favService = new UserFavoritesSerivce(userFavRepoDependency, someOtherDependency);
    var favs = favService.GetUserFavoritesServiceMethod();

    // add info
}

There might be a whole dependency chain you'll need to include. Testing and future changes here is more difficult. I'd now care much more about the search details since I've got to provide the other service's dependencies.

If the functions are closely related enough you might add the method implementation to the same class. Split the interface contracts so you can test these separately or if you want to limit access.

public class FavoritesService : IUserFavoritesService, IAddedInfoService
{
    private IRepository repo;

    // IUserFavoritesService
    public IEnumerable<UserFavorites> GetUserFavoritesServiceMethod()
    {
        var favs = GetFavorites();
    }

    // IAddedInfoService
    public FavoriteInfo GetSomeOtherInfoServiceMethod()
    {
        var favs = GetFavorites();
        // add info
    }

    private IQueryable<UserFavorites> GetFavorites()
    {
        var query = repo.Where(...);
    }
}

Only consume the methods you need to limit access and makes testing Consumer easier with a simpler mock.

public class Consumer
{
    private IAddedInfoService infoService;

    public Consumer(IAddedInfoService infoService)
    {
        this.infoService = infoService;
    }

    public FavoriteInfo GetInfo()
    {
        var info = infoService.GetSomeOtherInfoServiceMethod();
        return info;
    }
}

If the original service gets too big it'll be easier to refactor since Consumer didn't care about the IUserFavoritesService contract.

Then you might make a base class to provide the common functions.

public abstract class BaseFavService
{
    protected virtual IQueryable<UserFavorite> GetUserFavorites() {  }
}

public class UserFavorites : BaseFavService, IUserFavorites { }
public class AddedInfoFavoritesService : BaseFavService, IAddedInfoService { }

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.