RFC on "Factory" code This is the first code of this type that I've attempted, so I'm wondering if anybody would be willing to critique it. It's "sort of" (I think) a factory class. I want to keep it as simple as possible (and still work, and be at least as elegant as Phyllis Diller - it doesn't have to be Elizabeth Taylor).

The situation is that we may eventually support any number of printer types (printing labels from a Windows CE device), but want to keep the client code to something as simple as:

var beltPrinter = new BeltPrinter();
beltPrinter.PrintLabel(price, description, barcode);


Each print job will consist of 0..N preparatory commands sent to the printer ("hey, wake up!") followed by 1..N print commands, followed by 0..N shutdown commands.

Based on that, does the following make sense? If not, what would you advise that I change?

interface IBeltPrinter
    {
        private List&lt;string&gt; printerSetupCmds;
        private List&lt;string&gt; printLabelCmds;
        private List&lt;string&gt; printerTeardownCmds;

        void PrintLabel();
    }

class BeltPrinter : IBeltPrinter
{
    if (&lt;ZebraQL220&gt;) // don't know yet how I will determine which printer is in use - perhaps from reading a config file
    {
        return ZebraQL220Printer;
    }
    else if (&lt;ONeal&gt;) 
    {
        return ONealPrinter;
    }
}

class ZebraQL220Printer : IBeltPrinter
    {
        public void PrintLabel(string Price, string Desc, string Barcode)
        {
            var _price = Price;
            var _desc = Desc;
            var _barcode = Barcode;

            printerSetupCmds = new List&lt;string&gt;();
            printerSetupCmds.Add(''); // the format and commands, etc. will differ from other printers
            . . .
            printLabelCmds = new List&lt;string&gt;();
            printLabelCmds.Add('');
            . . .
            printerTeardownCmds = new List&lt;string&gt;();
            printerTeardownCmds.Add('');
            . . .

            foreach (string line in printerSetupCmds) {
    //send to ZebraQL220
            }
            foreach (string line in printLabelCmds) {
    //send to ZebraQL220
            }
            foreach (string line in printerTeardownCmds) {
    //send to ZebraQL220
            }
        }
    }

    class ONealPrinter : IBeltPrinter
    {
        public void PrintLabel(string Price, string Desc, string Barcode)
        {
            var _price = Price;
            var _desc = Desc;
            var _barcode = Barcode;

            printerSetupCmds = new List&lt;string&gt;();
            printerSetupCmds.Add(''); // the format and commands, etc. will differ from other printers
            . . .
            printLabelCmds = new List&lt;string&gt;();
            printLabelCmds.Add('');
            . . .
            printerTeardownCmds = new List&lt;string&gt;();
            printerTeardownCmds.Add('');
            . . .

            foreach (string line in printerSetupCmds) {
    //send to ONeal
            }
            foreach (string line in printLabelCmds) {
    //send to ONeal
            }
            foreach (string line in printerTeardownCmds) {
    //send to ONeal
            }
        }
    }
First off, I have to say this is a very clean and efficient design.

I'm hoping class IBeltPrinter BeltPrinter() is a typo, because I've never seen C# code like that.

Variable names for local variables (and parameters) should start with a lower case letter.

I would move the setup, labelCommands and tear down into their own methods.  It will keep the PrintLabel method much cleaner

public void PrintLabel(string price, string desc, string barcode)
{
    ExecuteSetup(/*variables*/);
    ExecuteCommands(/*variables*/);
    ExecuteTeardown(/*variables*/);
}


Come to think of it, you could have a base class that has the PrintLabel method, and virtualExecuteSetup,ExecuteCommands, andExecuteTeardown` methods, that have generic printer commands.  You could then extend that base class as required.

I would also consider putting the Factory definitions in an IDictionary.  This will eliminate having a potentially huge switch statement.

private readonly static IDictionary&lt;[whatever], IBeltPrinter&gt; SupportedPrinters = new Dictionary&lt;[whatever], IBeltPrinter&gt;
    {
        {[ZebraQL220], new ZebraQL220Printer()},
        {[ONeal], new ONealPrinter()},
    }


Then your factory call would be

public IBeltPrinter CreatePrinter([whatever] printer)
{
    if (!SupportedPrinters.ContainsKey(printer))
        throw new PrinterNotSupportedException(printer)

    return SupportedPrinters[printer];
}