C# Design Pattern for workers with different input parameters

Question

I'm not sure which design pattern might help me solve this issue.

I have a class, 'Coordinator', which determines which Worker class should be used - without having to know about all the different types of Workers there are - it just calls a WorkerFactory and acts upon the common IWorker interface.

It then sets the appropriate Worker to work and returns the result of its 'DoWork' method.

This has been fine... up until now; we have a new requirement for a new Worker class, "WorkerB" which requires an additional amount of information i.e. an additional input parameter, in order for it to do its work.

Its like we need an overloaded DoWork method with the extra input parameter...but then all the existing Workers would have to implement that method - which seems wrong as those Workers really dont need that method.

How can I refactor this to keep the Coordinator unaware of which Worker is being used and still allowing each Worker to get the information it requires to do its job but not have any Worker do things it doesn't need to?

There are a lot of existing Workers already.

I don't want to have to change any of the existing concrete Workers to accommodate the requirements of the new WorkerB class.

I thought maybe a Decorator pattern would be good here but I haven't seen any Decorators decorate an object with the same method but different parameters before...

Situation in code:

public class Coordinator
{
 public string GetWorkerResult(string workerName, int a, List<int> b, string c)
 {
 var workerFactor = new WorkerFactory();
 var worker = workerFactor.GetWorker(workerName);
 if(worker!=null)
 return worker.DoWork(a, b);
 else
 return string.Empty;
 }
}
public class WorkerFactory
{
 public IWorker GetWorker(string workerName)
 {
 switch (workerName)
 {
 case "WorkerA":
 return new ConcreteWorkerA();
 case "WorkerB":
 return new ConcreteWorkerB();
 default:
 return null;
 }
 }
}
public interface IWorker
{
 string DoWork(int a, List<int> b);
}
public class ConcreteWorkerA : IWorker
{
 public string DoWork(int a, List<int> b)
 {
 // does the required work
 return "some A worker result";
 }
}
public class ConcreteWorkerB : IWorker
{
 public string DoWork(int a, List<int> b, string c)
 {
 // does some different work based on the value of 'c'
 return "some B worker result";
 }
 public string DoWork(int a, List<int> b)
 {
 // this method isn't really relevant to WorkerB as it is missing variable 'c'
 return "some B worker result";
 } 
}

• JamesFaix: Is the IWorker interface listed the old version, or is that a new version with an added parameter?
• JamesFaix: Are the places in your code base that are currently using IWorker with 2 parameters going to need to plug the 3rd parameter in, or are only new call sites going to use the 3rd parameter?
• user22815: Instead of going shopping for a pattern, try focusing on the overall design regardless of whether or not a pattern applies. Recommended reading: How bad are "Shopping for Patterns"-type questions?
• Dunk: According to your code, you already know all the parameters needed before the IWorker instance is created. Thus, you should have passed those arguments to the constructor and not the DoWork method. IOW, make use of your factory class. Hiding the details of constructing the instance is pretty much the main reason for the factory class's existence. If you took that approach then the solution is trivial. Also, what you are trying to accomplish in the way you are trying to accomplish it is bad OO. It violates the Liskov Substitution Principle.
• Bernhard Hiller: I think you must go back another level. Coordinator already had to be changed to accommodate that extra parameter in its GetWorkerResult function - that means that the Open-Closed-Principle of SOLID is violated. As a consequence, all the code calling Coordinator.GetWorkerResult had to be changed also. So look at the place where you call that function: how do you decide which IWorker to request? That may lead to a better solution.

Answer 1

You will need to generalize the arguments so that they fit into a single parameter with a base interface and a variable number of fields or properties. Sort of like this:

public interface IArgs
{
 //Can be empty
}
public interface IWorker
{
 string DoWork(IArgs args);
}
public class ConcreteArgsA : IArgs
{
 public int a;
 public List<int> b;
}
public class ConcreteArgsB : IArgs
{
 public int a;
 public List<int> b;
 public string c;
}
public class ConcreteWorkerA : IWorker
{
 public string DoWork(IArgs args)
 {
 var ConcreteArgs = args as ConcreteArgsA;
 if (args == null) throw new ArgumentException();
 return "some A worker result";
 }
}
public class ConcreteWorkerB : IWorker
{
 public string DoWork(IArgs args)
 {
 var ConcreteArgs = args as ConcreteArgsB;
 if (args == null) throw new ArgumentException();
 return "some B worker result";
 }
} 

Note the null checks... because your system is flexible and late-bound, it is also not type safe, so you will need to check your cast to make sure the arguments that are passed are valid.

If you really don't want to create concrete objects for every possible combination of arguments, you could use a tuple instead (wouldn't be my first choice.)

public string GetWorkerResult(string workerName, object args)
{
 var workerFactor = new WorkerFactory();
 var worker = workerFactor.GetWorker(workerName);
 if(worker!=null)
 return worker.DoWork(args);
 else
 return string.Empty;
}
//Sample call
var args = new Tuple<int, List<int>, string>(1234, 
 new List<int>(){1,2}, 
 "A string"); 
GetWorkerResult("MyWorkerName", args);

• Machado: This is similar to how Windows Forms applications deal with events. 1 "args" parameter, and one "source of the event" parameter. All "args" are subclassed from EventArgs: msdn.microsoft.com/en-us/library/… -> I'd say that this pattern works very well. I just don't like the "Tuple" suggestion.
• Bernhard Hiller: if (args == null) throw new ArgumentException(); Now every consumer of an IWorker must know its concrete type - and the interface is useless: you can as well get rid of it and use the concrete types instead. And that's a bad idea, isn't it?
• John Wu: The IWorker interface is required due to the pluggable architecture (WorkerFactory.GetWorker can only have one return type). While outside the scope of this example, we know the caller is able to come up with a workerName; presumably it can come up with appropriate arguments as well.
• wondernate: Did you mean to put "if (ConcreteArgs == null)" ? Or if you did mean to put the "args" then shouldn't you test for that before you cast it?

Answer 2

I have re-engineered the solution based on @Dunk's comment:

...you already know all the parameters needed before the IWorker instance is created. Thus, you should have passed those arguments to the constructor and not the DoWork method. IOW, make use of your factory class. Hiding the details of constructing the instance is pretty much the main reason for the factory class's existence.

So I have shifted all the possible arguments required to create an IWorker into the IWorerFactory.GetWorker method and then each worker already has what it needs and the Coordinator can just call worker.DoWork();

 public interface IWorkerFactory
 {
 IWorker GetWorker(string workerName, int a, List<int> b, string c);
 }
 public class WorkerFactory : IWorkerFactory
 {
 public IWorker GetWorker(string workerName, int a, List<int> b, string c)
 {
 switch (workerName)
 {
 case "WorkerA":
 return new ConcreteWorkerA(a, b);
 case "WorkerB":
 return new ConcreteWorkerB(a, b, c);
 default:
 return null;
 }
 }
 }
 public class Coordinator
 {
 private readonly IWorkerFactory _workerFactory;
 public Coordinator(IWorkerFactory workerFactory)
 {
 _workerFactory = workerFactory;
 }
 // Adding 'c' breaks Open/Closed principal for the Coordinator and WorkerFactory; but this has to happen somewhere...
 public string GetWorkerResult(string workerName, int a, List<int> b, string c)
 {
 var worker = _workerFactory.GetWorker(workerName, a, b, c);
 if (worker != null)
 return worker.DoWork();
 else
 return string.Empty;
 }
 }
 public interface IWorker
 {
 string DoWork();
 }
 public class ConcreteWorkerA : IWorker
 {
 private readonly int _a;
 private readonly List<int> _b;
 public ConcreteWorkerA(int a, List<int> b)
 {
 _a = a;
 _b = b;
 }
 public string DoWork()
 {
 // does the required work based on 'a' and 'b'
 return "some A worker result";
 }
 }
 public class ConcreteWorkerB : IWorker
 {
 private readonly int _a;
 private readonly List<int> _b;
 private readonly string _c;
 public ConcreteWorkerB(int a, List<int> b, string c)
 {
 _a = a;
 _b = b;
 _c = c;
 }
 public string DoWork()
 {
 // does some different work based on the value of 'a', 'b' and 'c'
 return "some B worker result";
 }
 }

• Amorphis: you have a factory method that receives 3 parameters even though not all 3 are being used at all situations. what will you do if you have an object C that needs even more parameters? will you add them to the method signature? this solution is not extendable and ill advised IMO
• JTech: If I needed a new ConcreteWorkerC that needs more arguments, then yes, they would be added to GetWorker method. Yes, the Factory does not conform to the Open/Closed principal - but something somewhere has to be like this and the Factory in my opinion was best option. My suggestion is: rather than just saying this is ill advised, you'll help the community by actually posting an alternative solution.
• wondernate: This solution is not maintainable in the long run. If you have to update the parameters and this gets to have many cases then adding new ones or modifying old ones adds more and more risk. In the previous example, updates give you a nice separation of concern and straightforward implementations to unit test. That being said if you were looking for quick and dirty this solution will technically "work" but if you are going to use this in a production environment at a company, this is not very maintainable.
• JTech: @wondernate thanks for the comment. Yes agreed, as stated above, when we need new params, we'll need to modify WorkerFactory.GetWorker - breaking the open/closed principal, which I think has to happen somewhere and I believe restricting this to the Factory is the correct place. You state "updates give you a nice separation of concern and straightforward implementations to unit test" - ok great, please help the community by posting your solution. Otherwise, just saying the current answer isn't maintainable without providing an alternative solution, isn't of much value.
• wondernate: Thanks for the feedback @JTech ! I didn't offer an alternative because this isn't a post on unit testing so I didn't want to deviate from the topic. But as implied, this example is fine if it's a smaller set that is never going to be expanded. But for an enterprise environment, I would go with the previous example so I can add and test new implementations without touching the old ones and potentially have outdated parameters. I would refer people to other topics for actual examples of unit testing as that could go many different directions.

Answer 3

I would suggest one of several things.

If you want to maintain encapsulation, so that the callsites don't have to know anything about the inner workings of the workers or the worker factory, then you'll need to change the interface to have the extra parameter. The parameter can have a default value, so that some callsites can still just use 2 parameters. This will require that any consuming libraries are recompiled.

The other option I would recommend against, as it breaks encapsulation and is just generally bad OOP. This also requires that you can at least modify all the callsites for ConcreteWorkerB. You could create a class that implements the IWorker interface, but also has a DoWork method with an extra parameter. Then in your callsites attempt to cast the IWorker with var workerB = myIWorker as ConcreteWorkerB; and then use the three parameter DoWork on the concrete type. Again, this is a bad idea, but it is something you could do.

Answer 4

@Jtech, have you considered the use of the params argument? This allows a variable amount of parameters to be passed.

https://msdn.microsoft.com/en-us/library/w5zay9db(v=vs.71).aspx

• JTech: The params keyword might make sense if the DoWork method did the same thing with each argument and if each argument was of the same type. Otherwise, the DoWork method would need to check that each argument in the params array was of the correct type - but lets say we have two strings in there and each one was used for a different purpose, how could DoWork ensure that it has the correct one...it would have to assume based on position in the array. All too loose for my liking. I feel @JohnWu's solution is tighter.