I’m developing an app that will be used to open and close valves in an industrial environment, and was thinking of something simple like this:-
public static void ValveController
{
public static void OpenValve(string valveName)
{
// Implementation to open the valve
}
public static void CloseValve(string valveName)
{
// Implementation to close the valve
}
}
(The implementation would write a few bytes of data to the serial port to control the valve – an “address” derived from the valve name, and either a “1” or “0” to open or close the valve).
Another dev asked whether we should instead create a separate class for each physical valve, of which there are dozens. I agree it would be nicer to write code like PlasmaValve.Open() rather than ValveController.OpenValve("plasma"), but is this overkill?
Also, I was wondering how best to tackle the design with a couple of hypothetical future requirements in mind:-
- We are asked to support a new type of valve requiring different values to open and close it (not 0 and 1).
- We are asked to support a valve that can be set to any position from 0-100, rather than simply “open” or “closed”.
Normally I would use inheritance for this kind of thing, but I’ve recently started to get my head around “composition over inheritance” and wonder if there is a slicker solution to be had using composition?
10
If each instance of the valve object would run the same code as this ValveController, then it seems like multiple instances of a single class would be the right way to go. In this case just configure which valve it controls(and how) in the valve object’s constructor.
However if each valve control needs different code to run, and the current ValveController is running a giant switch statement that does different things depending on the type of valve, then you have reimplemented polymorphism poorly. In that case rewrite it to multiple classes with a common base(if that makes sense) and let the single responsibility principle be your design guide.
2
My major gripe is using strings for the parameter identifying the valve.
At least create a Valve class which has a getAddress in the form the underlying implementation needs and pass those to the ValveController and ensure that you can’t create nonexistent valves. This way you won’t have to handle wrong strings in each of the open and close method.
Whether you create convenience methods that call the open and close in ValveController is up to you, but to be honest I would keep all the communication to the serial port (including the encoding) in a single class which other classes will call when needed. This means that when you need to migrate to a new controller you only need to modify one class.
If you like testing you should also make ValveController a singleton so you can mock it (or create a training machine for the operators).
2