Factory pattern for creating embossing machines

Question

I'm working on a program that creates a file for a specific embosser machine (machine that creates plastic bank card for example). I have 3 embosser machines and all of them create different files. Every file contains a card number, last and first name, address, etc. The difference between the files is that they contain specific characters for the delimiter.

I want to implement this project with a factory pattern.

For simplicity sake, let's say that I have these embossers: DataCard, CIM and DataCard2.

This is how I created the files in my project, and I'd like to know if this is a good way of doing it.

public enum eEmbosserType
{
    Datacard,
    Cim,
    Datacard2
}

public interface IEmbosser
{
    eEmbosserType EmbosserType { get; }

    List<EmbosserData> Data { get; set; }

    void CreateFileForEmbosser(string path);

    void PrepareDataForEmboser();
}

   public class DataCardEmbosser : Emboser.IEmbosser
{

    public eEmbosserType EmbosserType => eEmbosserType.Datacard;

    public List<EmbosserData> Data { get ; set; }

    public void CreateFileForEmbosser(string path)
    {
        using (StreamWriter sw = new StreamWriter(path))
        {

            foreach (DataCard item in Data)
            {

                sw.Write(item.cardNumber + "_!");
                sw.Write(item.city + "_?");
                sw.Write(item.name + "_^");
                sw.Write(item.expDate + "_:");

            }

        }
    }

    public void PrepareDataForEmboser()
    {
        Data = new List<EmbosserData>();

        List<CardData> cards = GetCardData();

        foreach (CardData card in cards)
        {

            DataCard dc = new DataCard();

            dc.cardNumber = card.PAN;
            dc.city = card.City;
            dc.expDate = card.Expire;
            dc.name = card.Owner;

            Data.Add(dc);

        }

    }
}

public class CimEmbosser : Emboser.IEmbosser
{
    public eEmbosserType EmbosserType => eEmbosserType.Datacard2;

    public List<EmbosserData> Data { get; set; }

    public void CreateFileForEmbosser(string path)
    {
        using (StreamWriter sw = new StreamWriter(path))
        {

            foreach (Cim item in Data)
            {

                sw.Write(item.cardNumber + ";");
                sw.Write(item.city + ";");
                sw.Write(item.name + ";");
                sw.Write(item.expDate + ";");
                sw.Write(item.version + ";");

            }

        }
    }

    public void PrepareDataForEmboser()
    {
        Data = new List<EmbosserData>();

        List<CardData> cards = GetCardData();

        foreach (CardData card in cards)
        {

            Cim dc = new Cim();

            dc.cardNumber = card.PAN;
            dc.city = card.City;
            dc.expDate = card.Expire;
            dc.name = card.Owner;
            dc.version = DateTime.Now.ToString() + GetNewVersionNumber();

            Data.Add(dc);

        }
    }
}

public class DataCard2Embosser : Emboser.IEmbosser
{
    public eEmbosserType EmbosserType => eEmbosserType.Datacard2;

    public List<EmbosserData> Data { get; set; }

    public void CreateFileForEmbosser(string path)
    {

        using (StreamWriter sw = new StreamWriter(path))
        {

            foreach (DataCard2 item in Data)
            {

                sw.Write(item.cardNumber + ";_");
                sw.Write(item.city + "?");
                sw.Write(item.name + ";");
                sw.Write(item.expDate + "#");
                sw.Write(item.f1 + " ");
                sw.Write(item.f2 + " ");
                sw.Write(item.f3 + " ");

            }

        }

    }

    public void PrepareDataForEmboser()
    {
        Data = new List<EmboserData>();

        List<CardData> cards = GetCardData();

        foreach (CardData card in cards)
        {

            DataCard2 dc = new DataCard2();

            dc.cardNumber = card.PAN;
            dc.city = card.City;
            dc.expDate = card.Expire;
            dc.name = card.Owner;
            dc.f1 = card.PAN + " " + card.Expire;
            dc.f2 = card.Owner + " " + card.Address;
            dc.f3 = cards.Count().ToString().Trim();

            Data.Add(dc);

        }
    }
}

And factory creator

public class EmbosserCreator
{
    public static IEmbosser GetEmboser(eEmbosserType type)
    {

        switch (type)
        {
            case eEmbosserType.Datacard:
                return new DataCardEmbosser();
            case eEmbosserType.Cim:
                return new CimEmbosser();
            case eEmbosserType.Datacard2:
                return new DataCard2Embosser();
            default:
                throw new Exception("");
        }
    }
}

For data I thought I should create an abstract class, EmbosserData, which inherits classes DataCard, CIM, and DataCard2.

For example:

public abstract class EmbosserData
{
    public string cardNumber { get; set; }
    public string name { get; set; }
    public string city { get; set; }
    public string expDate { get; set; }
}

public class DataCard : EmbosserData
{

}

public class Cim : EmbosserData
{
    public string version { get; set; }
}

public class DataCard2 : EmbosserData
{
    public string f1 { get; set; }
    public string f2 { get; set; }
    public string f3 { get; set; }
}

Is this a good approach for my project? If you have any advice for other implementations, I'd love to know more.

PS. I hope that code is now ok to understand my idea.

Answer 1

eEmbosserType

• Don't use Hungarian Notation in C#. A better name would be EmbosserType or EmbosserKind.

IEmbosser

• This is a public interface, so I would expect that consumers could make their own implementations. If this is allowed, consumers are hampered by EmbosserType because this enum is under your control. Consider using an external key like string if more types are allowed.
• Next up are methods CreateFileForEmbosser, PrepareDataForEmboser and a mutable property Data. These are 3 separate concerns: IO, data lookup and in-memory data storage. This might be a bit too much for a single interface to handle. I feel these concerns are sufficiently independant to warrant separate interfaces to allow better reusability, granularity and modularity. Consider splitting into interfaces IDataRepository (db or any other data source access), IDataProvider (store the data in-memory with a read-only getter) and IDataWriter (write data to a stream).
• I have a few problems with method PrepareDataForEmboser since it's void and takes no parameters. Implementations only have the name of the method to decide what to do, and what is expected of them to do. From your implementatons I reverse engineered (should have documentation instead) that this method expects Data to be set. An additional problem here is that this property is mutable, allowing consumers to bypass the method and store Data directly, rendering your method unguarded and the design insufficiently encapsulated. Furthermore, implementations have to fetch the data GetCardData and store it internally, which is also mixing two different concerns.
• Method CreateFileForEmbosser is very specific that it needs to write to the system. I would change the specification to allow to write to a Stream instead. This way, consumers are free to provide the type of stream to write to, a file, a string builder, or any other stream.

class *Embosser

• I have questions about this design where each concrete instance fetches base class EmbosserData data using GetCardData, while iterating over concrete (for instance DataCard) data. I would have expected a base class to provide method GetCardData and perhaps also a generic type parameter for the return type.

EmbosserCreator

• In the initial revision of your question, you called this an Abstract Factory Pattern. Note, this is just a Factory Pattern. The former pattern is a factory for factories.
• Exception information might be a bit too cryptic for consumers and end-users: throw new Exception("");.

---

Conclusion

In general, I'm not sure why you need a factory for these different embossers, since each of them work with different data classes (probably also different data sources) and different data serialisation. Your question does not show us how you would use this pattern.