0

I am trying to implement Factory pattern in my application.

With reference to these links, I am trying to implement but stuck at one place & not sure how to proceed.

Please find "// confused here how do I implement here" comment in my code to get where I am stuck.

 //DAL Layer
 public interface IReportHandler
{
     IEnumerable<DocumentMapper> FetchDocumentsList(Guid clientId, int pager = 0);


}



public class ReportHandler : IReportHandler
{
      public IEnumerable<DocumentMapper> FetchDocumentsList(Guid clientId,  int pager = 0)
    {
          //implentation of the method
    }
}



//BLL 
public interface IReportFactory
{
    IReportHandler Create(int factoryId);
}

public class ReportFactory : IReportFactory
{
    private IReportHandler reportObj;

    public override IReportHandler Create(int factoryId)
    {
        switch (factoryId)
        {
            case 1:
                reportObj = new ReportHandler();
                return reportObj;
            default:
                throw new ArgumentException("");
        }


    }
}

//UI Layer
  public String GetAllDocuments(string url,int pager =0)
    {
        if (SessionInfo.IsAdmin)
        {
            string documents ; 
            //call GetDocumentIntoJson() private method 

        }
        else
        {
            return "Sorry!! You are not authorized to perform this action";
        }
    }


    private static string GetDocumentIntoJson(int clientId, int pager)
    {
       // confused here how do I implement here
        IReportHandler dal = ReportFactory
        var documents = dal.FetchDocumentsList(clientId, pager);
        string documentsDataJSON = JsonConvert.SerializeObject(documents);

        return documentsDataJSON;
    }

Can somebody guide me to implement the factory pattern + improve my code-snippet?

Any help/suggestion highly appreciated.

Community
  • 1
  • 1
Kgn-web
  • 7,047
  • 24
  • 95
  • 161
  • What's your original intent and what makes you think you need a Factory? – guillaume31 Dec 20 '16 at 11:02
  • I'm voting to close this question as off-topic because it has been also asked by the same person on code review and they have been given some very good answers there http://codereview.stackexchange.com/questions/150382/factory-for-report-handlers – Michael Coxon Dec 21 '16 at 02:22

3 Answers3

3

Many people implement the Factory pattern which is violating Open-Close principle. The implementation should be allowed to extension but close for changes.

The factory class should implemented to cater all the future extension.

Factory class:

 public class CalculatorFactory :ICalculatorFactory
    {
        private readonly IEnumerable<ICalculator> _calculators;

        public CalculatorFactory(IEnumerable<ICalculator> calculators)
        {
            _calculators = calculators;
        }
        public ICalculator GetCalculator(string type)
        {
            var calculator = _calculators.FirstOrDefault(t => t.Name == type.ToLower());
            if (calculator == null)
            {
                throw new NotImplementedException();
            }

            return calculator;
        }
    }

Startup:

 services.RegisterAllCalculatorTypes<ICalculator>(new [] {typeof(Startup).Assembly}, ServiceLifetime.Scoped);

Extension class for DI

 public static void RegisterAllCalculatorTypes<T>(this IServiceCollection services, Assembly[] assemblies,
            ServiceLifetime lifetime = ServiceLifetime.Transient)
        {
            var typesFromAssemblies = assemblies.SelectMany(a => a.DefinedTypes.Where(x => x.GetInterfaces().Contains(typeof(T))));
            foreach (var type in typesFromAssemblies)
            {
                services.Add(new ServiceDescriptor(typeof(T), type, lifetime));
            }
        }

Please check this video. It shows how to implement the Factory pattern without violating SOLID principles https://www.youtube.com/watch?v=BD-mZXOVTrU

Sam
  • 516
  • 3
  • 7
0

Your UI layer class need instance of ReportFactory

public class UIclass
{
    private readonly IReportFactory _reportFactory;

    public UIclass(IReportFactory reportFactory)
    {
        _reportFactory = reportFactory;
    }

    private string GetDocumentIntoJson(int clientId, int pager)
    {
        // Use factory to get a ReportHandler
        // You need provide "factoryId" from somewhere too
        IReportHandler dal = _reportFactory.Create(factoryId);

        var documents = dal.FetchDocumentsList(clientId, pager);
        string documentsDataJSON = JsonConvert.SerializeObject(documents);

        return documentsDataJSON;
    }
}
Fabio
  • 31,528
  • 4
  • 33
  • 72
  • Not sure why this is getting down voted since it is essentially correct. The only other way you could do it is stand the class up inside the method which would make the interface completely irrelevant. – Michael Coxon Dec 20 '16 at 11:14
  • Except for the `static` - which is odd and was in OP's code. – Michael Coxon Dec 20 '16 at 11:16
  • @MichaelCoxon, Can you please review my updated implemenation http://codereview.stackexchange.com/questions/150382/is-this-the-right-way-to-implement-factory-pattern – Kgn-web Dec 20 '16 at 12:34
  • Please review the same .http://codereview.stackexchange.com/questions/150382/is-this-the-right-way-to-implement-factory-pattern – Kgn-web Dec 20 '16 at 12:34
0
  1. Do not use such things:

    IReportHandler dal = new ReportFactory();

Because it makes useless your dependency on interface and create coupling to concrete realization. Instead use Dependency Injection container and inject such factories via constructor parameters or properties. Most popular DI containers are Castle Windsor, Ninject, Unity, Autofac etc.

If you don't want to use containers - at least create all your concrete implementations in one place in program entry point, register all implementations in Service Locator (read more about it) and pass Service Locator to hierarchy via constructors.

  1. Try to avoid static methods. Use interfaces instead. You want to have code which depends on abstractions and can be easy testable and mockable.
Dzianis Yafimau
  • 2,034
  • 1
  • 27
  • 38