2

I have the following code:

interface IService
{
    void Execute();
}

class ServiceA : IService
{
    public void Execute() { ... }
}

class ServiceB : IService
{
    public void Execute() { ... }
}

class ServiceComposite : IService
{
    List<IService> _services = new List<IService>();

    public ServiceComposite()
    {
        _services.Add(new ServiceA());
        _services.Add(new ServiceB());
    }

    public void Execute()
    {
        foreach (IService service in _services)
        {
            service.Execute();
        }
    }
}

The problem is that ServiceB depends on some results from ServiceA. My idea is to create container class for storing the results, then inject it into both ServiceA and ServiceB:

class ServiceResults
{
    public string SomeProperty {get; set;}
}

public ServiceComposite()
{
    ServiceResults result = new ServiceResults();
    _services.Add(new ServiceA(result));
    _services.Add(new ServiceB(result));
}

I am wondering if it is the best possible way to solve the problem. Maybe it breaks some principles or rules of which I don't know, or is simply "code smell". What are the better ways to do this?

Signum
  • 845
  • 1
  • 10
  • 33
  • 1
    I'd be interested to know more about why `B` needs results from `A` - it seems that if that's the case it's not right to treat them as two separate implementations. – Ant P Jul 20 '16 at 12:56
  • Or the service A can expose the results as a property and pass Service A as a dependency for Service B – Thangadurai Jul 20 '16 at 12:58
  • 1
    You're not actually implementing the "Composite" pattern here either. – Servy Jul 20 '16 at 13:09
  • @Servy, what makes you say that? – jaco0646 Jul 20 '16 at 14:36
  • @jaco0646 Because `IService` doesn't contain other `IService` references. – Servy Jul 20 '16 at 14:38
  • @Servy, it's one of the concrete implementations that contains multiple `IService` references, in this case: `ServiceComposite`. This conforms to the [pattern](https://sourcemaking.com/design_patterns/composite). – jaco0646 Jul 20 '16 at 14:40
  • @jaco0646 No, it doesn't. The composite pattern allows you to treat a single instance or a composite group of instances indistinguishably. The fact that there is a single object and a group of them composed doesn't mean it's following the composite pattern. It's when you can treat them indistinguishably that it's the composite pattern. – Servy Jul 20 '16 at 14:44
  • @Servy, you can treat them all indistinguishably. `ServiceA`, `ServiceB`, and `ServiceComposite` all implement the same interface: `IService`. Each of these three concrete classes implements the `Execute()` method, while two of them are single instances and the third is a group; but all three share one interface. – jaco0646 Jul 20 '16 at 15:30
  • 2
    @jaco0646 One instance composing two specific other implementations is different than being able to generically compose any number of any types of the objects, including those that are themselves composable of other objects, which are themselves composable of other objects, etc. The composite pattern is the representation of a tree, and this is not able to represent any tree. – Servy Jul 20 '16 at 15:37
  • @Servy, I see your point. We could trivially implement an `addIService()` method in the composite class to address this... or we could determine the composite design pattern doesn't really fit this problem. – jaco0646 Jul 20 '16 at 15:53
  • @jaco0646 Indeed. The OP may well not need the composite pattern, and that's fine, it's just confusing to say you're using it when you're actually not. – Servy Jul 20 '16 at 15:54
  • @Signum - Did this answer your question? – Gilad Green Jul 21 '16 at 08:43
  • @Servy sorry for late response, anyway thanks for your clarifications. You're right, I don't need composite pattern here, it's more like "aggregate" class which takes all services and runs them sequentially. – Signum Sep 08 '16 at 12:31

1 Answers1

1

If ServiceB needs ServiceA's result for it to function properly then why not have ServiceB depending on ServiceA?

public interface IService 
{
    void Execute();
}

public class ServiceA : IService
{
    public void Execute() { ... }
}

class ServiceB : IService
{
    public ServiceB(IService service)
    {
        Service = service;
    }

    public void Execute() { ... }

    public IService Servie { get; set; }
}

Then in the case that you execute all Services in a collection you can add a ServiceBase to make sure this service executes only once: (Full example)

On this basic implementation you can add: Async Execute, Thread-safe checking of the executing of InnerExecute, Flyweight factory for the generation of the specific IService, Have a ResponseBase with derived responses for each Service....

public class ServiceResponse { }
public interface IService
{
    ServiceResponse Execute();
}

public abstract class ServiceBase : IService
{
    public ServiceResponse Execute()
    {
        if (_response == null)
        {
            _response = InnerExecute();
        }
        return _response;
    }

    public abstract ServiceResponse InnerExecute();

    private ServiceResponse _response;
}

public class ServiceA : ServiceBase
{
    public override ServiceResponse InnerExecute()
    {
        return new ServiceResponse();
    }
}

public class ServiceB : ServiceBase
{
    public ServiceB(IServiceFactory serviceFactory)
    {
        ServiceFactory= serviceFactory;
    }

    public override ServiceResponse InnerExecute()
    {
        return ServiceFactory.GetServices(ServicesTypes.ServiceA).Execute();
    }

    public IServiceFactory ServiceFactory { get; set; }
}

And whoever uses these Services:

public enum ServicesTypes 
{
    ServiceA,
    ServiceB....
}

public interface IServiceFactory
{
    IEnumerable<IService> GetServices();

    IService GetServices(ServicesTypes servicesTypes);
}

public class SomeOtherThatExecuteServices
{
    public SomeOtherThatExecuteServices(IServiceFactory serviceFactory)
    {
        ServiceFactory = serviceFactory;
    }

    public IEnumerable<ServiceResponse> ExecuteServices()
    {
        return ServiceFactory.GetServices()
                             .Select(service => service.Execute());
    }

    public IServiceFactory ServiceFactory { get; set; }
}

Probably you will want to access the factory by some mapping key and probably you will want proper logic in the SomeOtherThatExecuteServices but all this will put you on the right way (+Using some proper IoC Container)

Graham
  • 7,431
  • 18
  • 59
  • 84
Gilad Green
  • 36,708
  • 7
  • 61
  • 95