Factory Pattern Understanding

Question

I have implementated the Factory Pattern as below.

However, since the individual classes are public, nothing prevents someone from instantiating them directly.

Is this correct? How do I ensure that the concrete classes are only created via the Factory?

namespace MRS.Framework
{
    public  abstract class DataSource
    {
        public override string ToString()
        {
            return "DataSource";
        }
    }

    public class XMLDataSource : DataSource
    {

    }

    public class SqlDataSource : DataSource
    {

    }

    public class CSVDataSource : DataSource
    {
        public int MyProperty { get; set; }


        public override string ToString()
        {
            return "CSVDataSource";
        }
    }
}

Factory implementation

namespace MRS.Framework
{
    public abstract class DataSourceFactory
    {
        public abstract DataSource CreateDataSource(DataSourceType datasourcetype);
    }

    public class CSVDataSourceFactory : DataSourceFactory
    {
        public CSVDataSourceFactory()
        {

        }
        public override DataSource CreateDataSource(DataSourceType datasourcetype)
        {
            return new CSVDataSource();
        }
    }


    public class XMLDataSourceFactory : DataSourceFactory
    {
        public override DataSource CreateDataSource(DataSourceType datasourcetype)
        {
            return new XMLDataSource();
        }
    }

    public class SqlDataSourceFactory : DataSourceFactory
    {
        public override DataSource CreateDataSource(DataSourceType datasourcetype)
        {
            return new SqlDataSource();
        }
    }

}

Main

 static void Main(string[] args)
        {
            DataSourceFactory datasourcefactory = new CSVDataSourceFactory();
            CSVDataSource ds = (CSVDataSource)datasourcefactory.CreateDataSource(DataSourceType.CSVDataSource);
            CSVDataSource myds = new CSVDataSource();
            Console.WriteLine(ds.ToString());
            Console.WriteLine(myds.ToString());
            Console.ReadLine();

        }

Answer 1

If you want to know how to prevent people from being able to create instances of the datasources you can do it through interfaces. Create an interface for each datasource with all of the method that you want it to expose. Make the class itself internal or a private inner class of the factory (internal is usually appropriate) and the interface public.

If the question is not how to stop people from creating instances but if you should, that's more of a subjective answer. There are a few things to consider:

1. What are the consequences of someone bypassing the factor and creating their own datasource? Nothing bad? Poorer performance? Major security violation? If they're only shooting themselves in the foot or hurting nothing, maybe you don't need to bother to stop them.
2. How hard is it to stop them? Do you need to prevent them from using reflection to access the underlying class? Is using an interface layer sufficient?
3. How trustworty are your users? Is it just an internal app for your own use or for use on your own team? Is it being shipped to thousands of other people? A small internal use app might not bother, a large scale one needs to assume there are either really malicious or ignorant users and what you expose will matter significantly. It is also important to improve usability in general as the userbase grows.

Answer 2

Yes, your intuition here is correct; if you want to restrict the construction of your CSVDataSourceFactory class then you have your access modifiers wrong.

However, it's not the access modifier of the class you need to fix, it's the access modifier of the constructor. You should mark the default constructor internal so that only other classes within your assembly can construct them. You will, of course, have to enforce your own rules within that assembly but since you have complete control over that code, it shouldn't be an issue.

public class XMLDataSource : DataSource
{
  internal XMLDataSource() { }
}

public class SqlDataSource : DataSource
{
  internal SqlDataSource() { }
}

public class CSVDataSource : DataSource
{
  public int MyProperty { get; set; }

  internal CSVDataSource() { }

  public override string ToString()
  {
      return "CSVDataSource";
  }
}