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:-

  1. We are asked to support a new type of valve requiring different values to open and close it (not 0 and 1).
  2. 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?

  • 2
    I would create a generic valve class which has an identifier for the specific valve (not a string, perhaps an enum) and any information necessary for control flow inside of the OpenValve/CloseValve methods. Alternatively you could make the valv class abstract and make separate implementations for each one, where open/close valve just calls the logic inside the given valve class for the event that different valves have different opening/closing mechanisms. The common mechanism would be defined in the base class. Oct 15, 2012 at 15:10
  • 2
    Don't worry about hypothetical future requirements. YAGNI.
    – pdr
    Oct 15, 2012 at 15:16
  • 3
    @pdr YAGNI is a double edged blade, I agree it's worth following in the general, but taken to the extreme one could say doing anything to help future maintainability or readability is violating YAGNI, because of this I find the scope of YAGNI too ambiguous for many. That said, many people recognize where to use YAGNI and where to toss it because accounting for the future will save you serious pain. I just think one should be careful suggesting folks follow YAGNI when you don't know where they'll land on that spectrum. Oct 15, 2012 at 15:23
  • 2
    Man, 'composition over inhertiance' is overrated. I'd make a Valve abstract class/interface and then subclass them into PlasmaValve. And then I'd make sure that my ValveController would work with Valve(s), not caring about which subclass they are exactly.
    – MrFox
    Oct 15, 2012 at 15:30
  • 2
    @suslik: Absolutely. I've also seen excellent code called spaghetti by people who don't understand SOLID principles. We could go on forever with this. My point is that I've seen more problems caused by dismissing established principles (born of many years' experience) out-of-hand than I've seen caused by over-adherence. But I agree that both extremes are dangerous.
    – pdr
    Oct 15, 2012 at 16:17

2 Answers 2


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.

  • 1
    +1 for mentioning type-based switch statements as code smell. I frequently see these kinds of switch statements where the developer claims he was just following KISS. Perfect example of how design principles can be perverted heh Oct 15, 2012 at 15:20
  • 2
    Multiple instances might also make it easier to link valves in a sequence, enabling you to model the actual plant piping as a directed graph in your code. You could also then add business logic to the classes, in case you need to do something like open one valve when another one closes to avoid pressure build-up, or close all downstream valves so you don't get a "water hammer" effect when the valve is opened again.
    – TMN
    Oct 15, 2012 at 15:48

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).

  • I have never seen anyone recommend a singleton for sake of testing before--usually it goes the other way.
    – Kazark
    Oct 18, 2012 at 16:42
  • honestly the singleton is more to avoid the statics and so the communication can be synchronized Oct 18, 2012 at 17:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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