4
\$\begingroup\$

Based on the post I made here: RFC on "Factory" code and its responses, and my (inner) response to those responses, I'm beginning to wonder if it would be just as well to simply switch based on the user's hardware, and forget about this "fancy pants" factory stuff. After all, new code will have to be added for each new printer we support, either the hard way (with all this abstraction) or the easy way ("just do it").

From what I understand, using the factory pattern is more "elegant," but still: new code has to be added; and apparently, because of the "open-closed principle" (open to extension, closed to modification), the elegant way is to continue to add new classes that inherit from prior ones. So we could end up with:

PrintClass, immutable after being written
NewPrintClass, "" (which inherits from PrintClass, but adds another printer type)
NewerPrintClass, "" (which inherits from NewPrintClass, but adds another printer type)
NewestPrintClass, "" ( etc. )
NewestestPrintClass, "" ( etc. )
etc. etc. ad infinitum ad nauseum

IOW, it might be best just to have this simple client code:

BeltPrinter.PrintLabel(price, description, barcode);

...and that would be implemented something like this:

namespace BeltPrinter;

    string _price = string.Empty;
    string _desc = string.Empty;
    string _barcode = string.Empty;
    List<string> printerSetupCmds = new List<string>();
    List<string> printLabelCmds = new List<string>();
    List<string> printerTeardownCmds = new List<string>();

public static void PrintLabel(string price, string description, string barcode) 
{
    _price = price;
    _desc = description;
    _barcode = barcode;

    if (<ZebraQL220>) // don't know yet how I will determine which printer is in use - perhaps from reading a config file...
    {
       PopulateListsForZebraQL220Printer();
    }
    else if (<ONeal>) 
    {
        PopulateListsForONealPrinter();
    }
    // add other calls to PopulateListsForX as needed later

    SendPrinterCodes();
}

private void PopulateListsForZebraQL220Printer()
{
    printerSetupCmds.Add(<add setup commands specific to Zebra>); 
    . . . 
    printLabelCmds .Add(<add print commands specific to Zebra, using _price, _desc, and _barcode>); 
    . . . 
    printerTeardownCmds .Add(<add teardown commands specific to Zebra>); 
    . . . 
}

private void PopulateListsForONealPrinter()
{
    printerSetupCmds.Add(<add setup commands specific to ONeal>); 
    . . . 
    printLabelCmds = new List<string>();
    printLabelCmds .Add(<add print commands specific to ONeal, using _price, _desc, and _barcode>); 
    . . . 
    printerTeardownCmds = new List<string>();
    printerTeardownCmds .Add(<add teardown commands specific to ONeal>); 
    . . . 
}

private void  SendPrinterCodes()
{
    ExecutePrintCommands(printerSetupCmds);
    ExecutePrintCommands(printLabelCmds);
    ExecutePrintCommands(printerTeardownCmds);
}

private void ExecutePrintCommands(List<string> printerCmdsList)
{
    foreach (string line in printerCmdsList) {
        serialPort.Write(line); // There's a lot of OpenNETCF stuff behind this line, this is just 'shorthand' for what happens
    }
}

I wouldn't exactly call that a maintenance nightmare*; it seems quite straightforward to me (of course, I wrote it, and within the last fifteen minutes, so YMMV), whereas the fancy pants "factory" stuff almost seems like a bunch of smoke and mirrors to me. It reminds me of the company attorney who came up with a convoluted legal agreement which the boss could not understand. The boss asked for explanation. Another employer put it plainly. The boss replied, "Would stating it in such a way, plainly and simply, have the same effect"? When the lawyer had to agree that yes, it would, he was soon seeking employment elsewhere.

  • For each new printer type, simply add another "else/if" block to PrintLabel(), and another PopulateListsForX() method to the class. And, of course, the printer-specific code, which is the bulk of the work in any case, and will need to be done whatever the underlying/enveloping programming milieu.

I'm not saying all these cats who are so much more clever than I, implementation-wise, are wasting their time or anybody else's. I'm just saying I'm not so sure the straightforward way is not just as good (at times, anyway).

I see it this way: What I will call "fancy pants coding" (using layers of abstraction, patterns, frameworks, the latest buzzword/acronym, etc.) is not in itself, in the long run, any easier or harder than "cowboy coding"; it's just that the hard part and the (relatively) easy part switch places. With fancy pants coding, the hard part comes first, in the architecture and design; with cowboy coding, the hard part is at the end, when you're adding on new and forgotten pieces and dealing with edge cases. So, which one you choose doesn't really matter (pick your poison); anyway, from my experience, you end up dealing with new and forgotten pieces and edge cases that are tough to navigate through no matter which methodology you practice or "philosophy" you espouse.

BUT (there's almost always a "but"): what about the maintenance part of it? In the specific case I'm dealing with, ISTM that adding new "PopulateListsForX()" methods as necessary (along with new "else if" blocks to the PrintLabel() method) would be much easier (read: faster) than extending classes after the canonical (fancy pants) fashion - which M.O. actually seems rather sloppy to me, like adding lean-tos onto the barn once your cow had another calf, or bedrooms off the kitchen after ... (etc.) To be fair, both ways of approaching it are rather like that, so again: it's sort of a tradeoff.

And, what I think is a key question (also based on empirical observation, having been involved in several projects for several companies): Which style of coding is easier for a "new guy" to hit the ground running with: the magnum opus/tour de force of the (possibly still around) boy genius architect, or the straightforward and easy-to-grok "plain vanilla" code?

The bottom line, as I see it, is that creating software is hard, really hard (unless, perhaps, you are a genius, which I obviously am not). As Brooks (not Foster, not Robert: Frederick) said, there are no silver bullets.

\$\endgroup\$
  • 6
    \$\begingroup\$ Tip: Reduce the amount of text. The tl;dr-factor here is fairly high. \$\endgroup\$ – Lstor Jul 25 '13 at 19:39
  • \$\begingroup\$ Yes, it makes everyone dizzy at first, but it quickly becomes second nature, like riding a bike. I would recommend creating a SettingsReader business class of some sort which determines which printer to use. The factory could then create one of those and use it to set it's enum field. \$\endgroup\$ – Steven Doggart Jul 26 '13 at 22:44
  • \$\begingroup\$ hmmm, I'm not sure how this could work as _printerChoice never seems to be set, or provided to the factory.... Did you have some other "fancy pants" way of setting that :) \$\endgroup\$ – dreza Jul 29 '13 at 20:13
  • \$\begingroup\$ For the moment, I'm simply setting it to Zebra, as it's the only one we support at present. In the future, will have to read a config file or something to determine which belt printer to target. Besides, the switch statement returns zebra as a fallback/default. \$\endgroup\$ – B. Clay Shannon Jul 29 '13 at 20:35
  • 1
    \$\begingroup\$ This doesn't seem to really be a code review question. \$\endgroup\$ – Winston Ewert Jul 29 '13 at 22:34
8
\$\begingroup\$

To quote an ancient truth, "To every thing there is a season, and a time to every purpose." There is a time to fancy-pants-code, and there is a time to cowboy-code. Experience is what has to guide you in determining which method is better suited for a given task. There are many things that can impact that decision, for instance:

  • How long will it take
  • How much time do I have until it must be done
  • How long will this code be around
  • How often is the code going to need modifications in the future
  • How many other developers will need to work on this code

Simply put, from an idealistic standpoint, fancy-pants code is always better than cowboy code, but from a practical standpoint, fancy-pants code can sometimes be much worse. So, as a general rule-of-thumb, I would say that you should always fancy-pants-code unless you have a good reason not to do so, and you should accept that there are some very good reasons not to do so, sometimes.

I have a lot of experience with this, and I can tell you, designing code well up-front, is WELL worth the effort. It's true, as you said, that good design has more up-front cost, and poor design has more back-end cost, but to say that the two are equivalent, because of that, is fallacious. It all depends how much back-end work will need to be done.

As you say, a simple switch statement isn't really that bad, and if adding more case statements to the switch statement is the worst of the back-end cost that you are going to incur, then it's really not worth the time to add a bunch of complication to avoid something as trivial as that. But if the project grows and changes over the years, a little shortcut like that, to save a few hours of development time, could potentially cost months of development time in the long run. It all depends.

Another issue which often muddies the water is that not all fancy designs are actually good designs. You could develop the most impressive fancy code in the world, but if it doesn't actually solve any problems, such as making the code more stable, flexible, or maintainable, then you've actually made the problem worse by all your fanciness.

As much as I am a believer of good design, I am also a strong believer in the YAGNI principle. If you haven't read the wikipedia page on YAGNI, I'd strongly recommend it. YAGNI simply means "You aint gonna need it". Simply put, don't add features and complication that you don't currently need. YAGNI does not preclude good design, but it does preclude unnecessary fanciness. Again, the issue isn't whether or not the design is fancy, it's whether or not the design is good. A good design will follow the YAGNI principle. It is definitely possible to "over-design" something.

All that being said, if there is no good reason to cowboy-code on this one, as it sounds like their might not be, allow me to offer my perspective on what a good fancy design might look like. My way is by no means the only right way, but hopefully it will be helpful to you. I'd like to think that my fancy way of doing it does solve a lot of future problems and would therefore be worth the relatively small up-front cost.

My Way to Do It (Dependency-Injection)

Your first instinct was to think that the Factory Pattern would be a good solution to this problem. That was a good instinct, but I don't think it quite goes far enough. Dependency-Injection (DI), which relies heavily on factories, is perfectly suited to this type of problem. Personally, I think it is well-suited for almost all problems, but there are certain problems, like this one, which just scream dependency-injection.

In general, the principle of DI is that a class never creates it's own dependencies. So, for instance, if you have a Car class, and it needs an Engine object, you wouldn't have the Car class create its own Engine object. Instead, you would have the Car class request that the Engine (it's dependency) be given to it (injected), usually in its constructor. For example, rather than having something like this:

public class Car
{
    private Engine _engine = new Engine();
}

With DI, you'd have something like this:

public class Car : ICar
{
    public Car(IEngine engine)
    {
        _engine = engine;
    }

    private IEngine _engine;
}

When you think about it, it makes perfect logical sense. In the real world, you'd never even think of designing a car that creates it's own engine--that would be a nightmare. You of course would build a factory which would create both the car and then engine, and then put the engine into the car (inject it). The equivalent in code, would be to create a factory class, like this:

public class Factory
{
    public ICar NewCar()
    {
        return new Car(new Engine());
    }
}

The advantage of doing this is that, just as in real life, you have made the situation much less complicated, and much more flexible, by separating the business logic (i.e. how the car actually functions internally, what it does, or, more simply, what makes it a car), from the construction logic (i.e. how to build the individual parts and how to wire them all together).

You'll notice that in the DI example, the car and engine implement interfaces. Interfaces are very important in dependency injection. The point is, that as long as all engines implement the same public interface, the car shouldn't care which engine is actually injected into it. As long as they all serve the same purpose and are called in the same way, their internal workings are irrelevant to the car. In other words, as long as when you press the accelerator, the engine speeds up, it doesn't really matter if it's a jet engine, an internal combustion engine, or a hamster in a hamster wheel. In other words, the car's engine is plug-and-play.

Their are many advantages to this methodology. For instance, DI is used heavily by unit-testing enthusiasts because it makes it very easy to test each class in isolation. For instance, if you want to test the car class, you are in full control of what you give it as an engine. You don't even have to give it a real engine at all. You could give it a fake (mock) engine that just reports back to the unit tester whether or not the car was sending the correct signals to the engine at the right times.

Another major advantage of DI is that it avoids spaghetti code and buggy-ness in the long-run. For instance, lets say, in the future, you keep adding more and more different kinds of engines to your code. With DI, you'll possibly never even have to touch a line of code in your car class to do so. The less you have to modify the code, the less bugs you will create. The alternative is to have, right in the middle of your business logic, a whole bunch of construction logic which keeps growing in complexity with each new engine type that you add.

Your Project, the DI Way

So, what does this all mean to you in your situation? Here is a basic idea of how I would implement it with DI principles. First, I would create a simple interface which would be common for all of your printers:

public interface IBeltPrinter
{
    void PrintLabel(string price, string description, string barcode);
}

Then, I would create a separate concrete implementation for each type of printer, like this:

public class ZebraQL220Printer : IBeltPrinter
{
    public void PrintLabel(string price, string description, string barcode)
    {
        // Do it the Zebra way
    }
}

public class ONealPrinter : IBeltPrinter
{
    public void PrintLabel(string price, string description, string barcode)
    {
        // Do it the ONeal way
    }
}

The beauty here is, you are free to implement these printer classes however you want. If some of them all work very similarly internally, you could create a base class for them that those classes derive from. But if you need to make a new printer class, sometime in the future, which doesn't work the same way at all, you're free to implement it anyway you want. As long as it implements the IBeltPrinter interface, that's all that matters. It doesn't matter what class it derives from, how it works, what namespace it exists in, or even what library it comes from. So, for instance, if you do have a bunch of printers that all work by sending the same kinds of basic commands, you could create a base class, like this:

public abstract class CommandDrivenBeltPrinter : IBeltPrinter
{
    public void PrintLabel(string price, string description, string barcode)
    {
        foreach (string i in GetSetupCommands()) ExecuteCommand(i);
        foreach (string i in GetPrintCommands(price, description, barcode)) ExecuteCommand(i);
        foreach (string i in GetTeardownCommands()) ExecuteCommand(i);
    }

    private void ExecuteCommand(string command)
    {
        // Do it in the way that is common to all command-driven printers
    }

    protected abstract List<string> GetSetupCommands();
    protected abstract List<string> GetPrintCommands(string price, string description, string barcode);
    protected abstract List<string> GetTeardownCommands();
}

Then, you could implement all of your command-driven printers using that same base class, like this:

public class ZebraQL220Printer : CommandDrivenBeltPrinter
{
    protected override List<string> GetSetupCommands()
    {
        return new List<string>(new string[] {"Zebra command 1", "Zebra command 2"});
    }

    protected override List<string> GetPrintCommands(string price, string description, string barcode)
    {
        return new List<string>(new string[] { "Zebra command 3", "Zebra command 4" });
    }

    protected override List<string> GetTeardownCommands()
    {
        return new List<string>(new string[] { "Zebra command 5", "Zebra command 6" });
    }
}

public class ONealPrinter : CommandDrivenBeltPrinter
{
    protected override List<string> GetSetupCommands()
    {
        return new List<string>(new string[] { "ONeal command 1", "ONeal command 2" });
    }

    protected override List<string> GetPrintCommands(string price, string description, string barcode)
    {
        return new List<string>(new string[] { "ONeal command 3", "ONeal command 4" });
    }

    protected override List<string> GetTeardownCommands()
    {
        return new List<string>(new string[] { "ONeal command 5", "ONeal command 6" });
    }
}

But again, the point is, you don't have to use that base class at all. At that point it's totally up to you how you implement each printer class. They may all share the same base class, there may be three different base classes, or you could implement them all separately with no shared inheritance at all. In fact, rather than using inheritance, you may want to create a separate helper class which includes all the common business logic, and then just inject it into each of the printer classes that need it, for instance:

public interface ICommandDrivenPrinterBusiness
{
    private void ExecuteCommand(string command);
}

public class CommandDrivenPrinterBusiness : ICommandDrivenPrinterBusiness
{
    private void ExecuteCommand(string command)
    {
        // Do it in the way that is common to all command-driven printers
    }
}

public class ZebraQL220Printer : IBeltPrinter
{
    public ZebraQL220Printer(ICommandDrivenPrinterBusiness business)
    {
        _business = business;
    }

    private ICommandDrivenPrinterBusiness _business;

    public void PrintLabel(string price, string description, string barcode)
    {
        _business.ExecuteCommand("Zebra command 1");
        _business.ExecuteCommand("Zebra command 2");
    }
}

Then, you would need to create a separate factory class which creates all of these objects and wires them together properly:

public interface IBeltPrinterFactory
{
    IBeltPrinter NewBeltPrinter();
}

public enum BeltPrintersEnum
{
    ZebraQL220,
    ONeal
}

public class BeltPrinterFactory : IBeltPrinterFactory
{
    private BeltPrintersEnum _printerChoice;

    public IBeltPrinter NewBeltPrinter()
    {
        switch (_printerChoice)
        {
            case BeltPrintersEnum.ZebraQL220: return new ZebraQL220Printer();
            case BeltPrintersEnum.ONeal: return new ONealPrinter();
        }
    }

    private IBeltPrinter NewZebraQL220Printer()
    {
        return new ZebraQL220Printer();
    }

    private IBeltPrinter ONealPrinter()
    {
        return new ONealPrinter();
    }
}

Then, when you actually need to print a label, you could just do it like this:

public class OrderBusiness
{
    public void ProcessOrder(OrderInfo order)
    {
    // ...
    IBeltPrinterFactory factory = new BeltPrinterFactory();
    IBeltPrinter printer = factory.NewBeltPrinter();
    printer.PrintLabel(order.price, order.description, order.barcode);
    // ...
    }
}

Or even better yet, let the DI spread, like a really pleasant virus:

public class OrderBusiness
{
    public OrderBusiness(IBeltPrinter printer)
    {
        _printer = printer
    }

    IBeltPrinter printer;

    public void ProcessOrder(OrderInfo order)
    {
    // ...
    _printer.PrintLabel(order.price, order.description, order.barcode);
    // ...
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ "Another issue which often muddies the water is that not all fancy designs are actually good designs." Boy howdy! I've seen (up close) way too many projects where the architect knew just enough to be dangerous and served inadvertently (I assume it was inadvertently!) as agents provacateurs, sabotaging the project or even the company by means of their "wizardry." \$\endgroup\$ – B. Clay Shannon Jul 26 '13 at 17:09
  • \$\begingroup\$ :) Yes, I've seen my share of that too. Sometimes it can be dangerous when someone who doesn't fully understand the fundamentals starts wanting to use every newfangled methodology or technology. Not because it's better or well suited to the task, but simply because its new. \$\endgroup\$ – Steven Doggart Jul 26 '13 at 18:00
  • 1
    \$\begingroup\$ This is a great article...I mean response (hint, hint). I had never quite grokked DI until now. In fact, I am preparing to pull on my fancy pants. \$\endgroup\$ – B. Clay Shannon Jul 26 '13 at 18:19
  • \$\begingroup\$ And yes, I am quite familiar with both Ecclesiastes and The Byrds. \$\endgroup\$ – B. Clay Shannon Jul 26 '13 at 18:24
  • \$\begingroup\$ Thanks. One of my favorite books, actually (and not a bad song either). \$\endgroup\$ – Steven Doggart Jul 26 '13 at 18:41
7
\$\begingroup\$

Start from the simplest possible way to solve the problem, and add complexity if there is a reason. Reasons could be a specific need from the specifications, or something that your experience tells you will very likely be needed at some point.

In this specific case, there would be a point in isolating the implementation for a printer inside a class by itself. That way you can specify an interface for what the class needs to implement, which would make it clear what needs to be implemented to anyone implementing another class.

The interface would also allow the client code to use any printer implementation without having to know what printer it is, or having separate code for separate printers.

To determine which implementation to use, a row of if statements (or a switch) is the simplest to follow. It needs to be maintained along with the classes, of course, but it's easier for a newcomer to see what's happening, compared to any fancy way of registering classes automagically.

\$\endgroup\$
  • \$\begingroup\$ I think we differ on the "when there is a reason" point (that is to say, point in time - for my specific case). \$\endgroup\$ – B. Clay Shannon Jul 25 '13 at 20:32
  • \$\begingroup\$ @ClayShannon: Good point, I changed it to "if there is a reason", as I of course meant the complexity added initially. \$\endgroup\$ – Guffa Jul 25 '13 at 20:41
  • 1
    \$\begingroup\$ Almost exactly what I'd answer. But this is my cue to pull out my favourite pet peeve: that many (most) languages supports interfaces over multiple inheritance, which (used right) is a pragmatic way writing code that is still reusable, without getting caught up in too many layers. \$\endgroup\$ – Michael Zedeler Jul 25 '13 at 22:24
  • \$\begingroup\$ You must be a C++ cat (not that there's anything wrong with that). \$\endgroup\$ – B. Clay Shannon Jul 25 '13 at 23:19
  • \$\begingroup\$ Well, not really. JavaScript and Perl. \$\endgroup\$ – Michael Zedeler Jul 25 '13 at 23:33

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