0

I have such interface:

public interface IImportModel
{
}

And class which implements this interface:

public class MaterialImportModel: IImportModel
{
    public string Name { get; set; }
}

Also I have interface for processors of import:

public interface IImportProcessor<TModel> where TModel: IImportModel
{
    void Process(TModel model);
}

And there is one of implementation of this interface:

public class MaterialImportProccessor : IImportProcessor<MaterialImportModel>
{
    public void Process(MaterialImportModel model)
    {
        // do some logic here
    }
}

Now I want to create Factory to instantiate such kind of processors. I have interface:

public interface IImportProcessorFactory
{
    IImportProcessor<IImportModel> Get(Parameter parameter);
}

And I'm trying to create implementation:

    public class ImportProcessorFactory : IImportProcessorFactory
    {
        public IImportProcessor<IImportModel> Get(Parameter parameter)
        {
             switch (Parameter) 
             {
                case "Materials":
                     IImportProcessor<IImportModel> processor = new MaterialImportProccessor();
                     return processor;

                case "Companies":
                    IImportProcessor<IImportModel> processor = new CompaniesImportProccessor();
                    return processor;
             }
        }
    }

But I got exception:

Error CS0266 Cannot implicitly convert type 'MaterialImportProccessor' to IImportProcessor<IImportModel>. An explicit conversion exists (are you missing a cast?)

It's correct. But I can't use out keyword to make IImportProcessor covariance because I use TModel as input parameter of Process(TModel method) method.

Are there any ways to refactor this code to make it work?

EDITED

I decided to give additional information about how I plan to use this factory.

var deserializer = _deserializerFactory.Get(/*some parameters*/);
var importProcessor = _importProcessorFactory.Get(someEunmValue);

var data = deserializer.Deserialize(file);
importProcessor.Process(data);

Because of it I can't make import processor factory generic.

Vlad B
  • 1
  • 3
  • did you try to add an cast? – Rampage64 Nov 09 '19 at 07:57
  • Yes, I've tried. But there is a runtime error in this case. – Vlad B Nov 09 '19 at 07:58
  • Coz `IImportProcessor` is not implemented by `MaterialImportProccessor` class. – Chetan Nov 09 '19 at 08:32
  • Yes, but MaterialImportProccessor implements IImportProcessor, where MaterialImportModel implements IImportModel. It can be work if use 'out' keyword. For example: public interface IImportProcessor where TModel: IImportModel; But I can't do it, because TModel is input parameter for 'Process()' method – Vlad B Nov 09 '19 at 08:51

5 Answers5

0

You Should Change to:

public interface IImportModel
{

}

public class MaterialImportModel: IImportModel
{
    public string Name { get; set; }
}

public class CompanieImportModel: IImportModel
{
    public int Id { get; set; }
    public string Name { get; set; }
}

public interface IImportProcessor
{
     void Process(IImportModel model);
}

public class MaterialImportProccessor : IImportProcessor
{
    public void Process(IImportModel model)
    {
        var obj = (MaterialImportModel) model;

        // do some logic here
    }
}

public interface IImportProcessorFactory
{
    IImportProcessor Get();
}


public class ImportProcessorFactory 
{
    public IImportProcessor Get(Parameter parameter)
    {
         switch (Parameter) 
         {
            case "Materials":
                 IImportProcessor processor = new MaterialImportProccessor();
                 return processor;

            case "Companies":
                IImportProcessor processor = new CompaniesImportProccessor();
                return processor;
         }
    }
}
Mahdi Asgari
  • 272
  • 1
  • 13
  • In this case I have to implement new factory for each Import model. Because I should import not only Materials, but Companies, Users etc... – Vlad B Nov 09 '19 at 08:29
  • I've edited my question to explain why I can't use generic factory. Thanks! – Vlad B Nov 09 '19 at 09:03
  • if you can't use Generic class you can use Generic method `IImportProcessor Get() where TModel: IImportModel` – Mahdi Asgari Nov 09 '19 at 09:46
  • No, I can't use generic method as well. Because on step where I try to get import processor I don't know type to use it in Generic method. I have only enum. And I want to get concrete import processor by this enum from factory. – Vlad B Nov 09 '19 at 09:54
  • So you shouldn't use Generic, you can cast your interface to class in MaterialImportProccessor.Process method – Mahdi Asgari Nov 09 '19 at 10:04
  • Yes, I will have to do it in case if I will not able to find better solution. Just wanted to avoid cast in each processor. Thanks! – Vlad B Nov 09 '19 at 10:06
  • You can't cast a class to an interface you should cast an interface to a class. https://stackoverflow.com/questions/10833946/how-to-cast-from-a-concrete-class-to-an-interface-type – Mahdi Asgari Nov 09 '19 at 10:12
0

UPDATE

https://dotnetfiddle.net/HYtWiN

consider using dynamic keyword for your factory:

    public interface IImportProcessorFactory 
    {
        dynamic Get(Parameter parameter);
    }

    public class ImportProcessorFactory : IImportProcessorFactory
    {
        public dynamic Get(Parameter parameter)
        { 
            switch (parameter)
            {
                case Parameter.Materials:
                    return new MaterialImportProccessor() ; 

                case Parameter.Companies:
                    return new CompaniesImportProccessor();
                default:
                    return null;
            }
        }
    }

so you are able to use it similar to this:

        var factory = new ImportProcessorFactory();
        var material = factory.Get(Parameter.Materials);
        var company = factory.Get(Parameter.Companies);


        var model = new MaterialImportModel();
        model.MaterialName = " Metal ";
        material.Process(model);

        var cModel = new CompanyImportModel();
        cModel.CompanyName = "Build Metal Company";
        company.Process(cModel);

enjoy!

Rampage64
  • 158
  • 1
  • 6
  • Factory should return not only MaterialImportProcessor. It should be generic. Because I will have a lot of import processors. Because of it I can't restrict factory to use MaterialImportModel. – Vlad B Nov 09 '19 at 08:46
  • Thanks. But I wanted to avoid generic factory. Because on step when I try to get import processor I don't know concrete type of import model. I have only enum. And factory should return me concrete import processor by this enum. I edited my question, and gave additional information about how I plan to use factory. – Vlad B Nov 09 '19 at 09:38
  • @VladB check again now, I modified the implementation of the `MaterialImportProccessor` class and simplified it – Rampage64 Nov 09 '19 at 10:14
  • Thanks. I knew about solution with casting. But I wanted to avoid this. I will have to go with it if I will not able find better solution. Also I need generic IImportProcessor because I plan to register all import processors in DI container. Because of this they should be parameterized. Otherwise I will able to register only one. – Vlad B Nov 09 '19 at 10:22
  • @VladB then consider to use dynamic, I updated the fiddle url again hope it helps this time :) – Rampage64 Nov 09 '19 at 11:13
  • Looks good! Thank you very much. I provided solution with additional abstract class and 'as' operator. You can read it. I will think about these two solutions. Now I don't know a lot about dynamics. But it looks almost like I wanted! – Vlad B Nov 09 '19 at 11:23
  • @VladB good to hear that! do not forget then to mark the correct answer :) Cheers – Rampage64 Nov 09 '19 at 11:27
0

Update 2

This the best I can today.

public interface IImportProcessor
{
     void Process();
}

public class ImportModel1 { public string Name { get; set; }}

public class ImportProcessor1 : IImportProcessor
{
    private readonly ImportModel1 data;
    public ImportProcessor1(ImportModel1 data) { this.data = data;}
    public void Process() => Console.WriteLine($"My name is {data.Name}");
}

public class ImportModel2{ public int Age { get; set; }}

public class ImportProcessor2 : IImportProcessor
{
    private readonly ImportModel2 data;

    public ImportProcessor2(ImportModel2 data) { this.data = data;}
    public void Process() => Console.WriteLine($"My age is {data.Age}");
}

public enum EType {One = 1, Two = 2}


public class ImportProcessorFactory
{
    public IImportProcessor Get(EType type, string file) 
    {   
        switch (type)
        {
            case EType.One: return new ImportProcessor1(JsonConvert.DeserializeObject<ImportModel1>(file));
            case EType.Two: return new ImportProcessor2(JsonConvert.DeserializeObject<ImportModel2>(file));
        }

        throw new NotImplementedException("Unable to work with '{type}'");
    }
}


class Program
{

    public static void Main() 
    {
        var f = new ImportProcessorFactory();

        var data1 = (EType.One, "{ Name: 'Vlad' }");
        var p1 = f.Get(data1.Item1, data1.Item2 );
        Console.WriteLine(p1.GetType());
        p1.Process();

        var data2 = (EType.Two, "{ Age: '20' }");

        var p2 = f.Get(data2.Item1, data2.Item2 );
        Console.WriteLine(p2.GetType());
        p2.Process();


    }
}

Output

ImportProcessor1
My name is Vlad
ImportProcessor2
My age is 20

Update 1

Drop the model interface and make the processor take T

public interface IImportProcessor<T>
{
    void Process(T model);
}

With this setup things should hold together.

public interface IImportProcessor<T>
{
    void Process(T model);
}

public class MaterialImportModel
{
    public string Name { get; set; }
}

public class MaterialImportProccessor : IImportProcessor<MaterialImportModel>
{
    public void Process(MaterialImportModel model)
    {
        // do some logic here
    }
}

public interface IImportProcessorFactory<T>
{
    IImportProcessor<T> Get();
}

public class ImportProcessorFactory : IImportProcessorFactory<MaterialImportModel>
{
    public IImportProcessor<MaterialImportModel> Get() => new MaterialImportProccessor();
}

(Old answer)

I think making IImportProcessor non generic would make your model much simpler and removed a whole class of problems.

public interface IImportProcessor
{
    void Process(IImportModel model);
}

The material processor should implement Process that takes the interface, not a concrete class. Otherwise it a) doesn't meet the contract and b) you getting rid of the benefits of interfaces :).

public class MaterialImportProcessor : IImportProcessor
{
    public void Process(IImportModel model)
    {
        // do some logic here
    }
}
tymtam
  • 31,798
  • 8
  • 86
  • 126
  • Yes. I agree. If I will not able to find other solutions I will have to use this approach. But in this case I have to cast IImportModel to concrete class in each processor which I wanted to avoid. Also I can use just object. Thanks for help. – Vlad B Nov 09 '19 at 09:12
  • If you have to cast then why not stop pretending that there are interfaces here? Just use concrete classes. – tymtam Nov 09 '19 at 09:17
  • If you're casting for `Name` then make it part of the model interface. – tymtam Nov 09 '19 at 09:18
  • I added interface just in attempt to write generic factories. I want to have factory which will return concrete import processor by some input parameters. I will have a lot of models to import, and each of them works with concrete type. In factory I can't use concrete type. – Vlad B Nov 09 '19 at 09:26
  • "generic factories" - generic != interfaces. Let's make it generic. – tymtam Nov 09 '19 at 09:30
  • @VladB I've updated my answer see at the fiddle link – Rampage64 Nov 09 '19 at 09:33
  • Yes, I know. But my factory should return some base class. Because of it I added interface. I can use abstract class instead of this but it does nothing in my case. Also I can't make factory generic (I described it in question). But it still should return generic import processor. Perhaps I should refactor this code to achieve this goal. But I don't know how. If you know tell me, please. – Vlad B Nov 09 '19 at 09:43
  • If I understood your example correct I should create new factory for each import processor. But I want to create only one factory which will create appropriate import processor by enum value which I will pass to Get() method of factory. – Vlad B Nov 09 '19 at 09:48
  • I can't send data through constructor because I need to register import processors in DI container. Also each import processor can have its own dependencies. – Vlad B Nov 09 '19 at 11:08
  • Confused. I thought we're writing the factory. If the possible processors are already available and we just have to ask for one depending on type then this was a waste of time... – tymtam Nov 09 '19 at 11:12
  • I'm planing to register all import processors in DI container.(Because of this they should be generic to implement different interfaces). After that import processor factory using DI container schould return them by some enum. I've simplified my question to avoid extra information. Anyway I should resolve problem which I described. I guess it doesn't relate to the fact weather we use DI container inside or not. Sorry for misunderstanding. – Vlad B Nov 09 '19 at 12:24
0

I decided to add additional abstract class:

public abstract class ImportProcessor<TModel> : IImportProcessor<MaterialImportModel> where TModel: class
{
    public void Process(IImportModel model)
    {
        Process(model as TModel);
    }

    public abstract void Process(TModel model);
}

and modified MaterialImportProccessor

public class MaterialImportProccessor : ImportProcessor<MaterialImportModel>
{
    public override void Process(MaterialImportModel model)
    {

    }
}
Vlad B
  • 1
  • 3
-1

Did you try to add a cast like user DevFlamur mentioned, using the as operator?

Mahesh Nayak
  • 104
  • 5
  • When I try to use 'as' operator factory returns null instead of instance of processor. When I use explicit cast I get runtime exception. – Vlad B Nov 09 '19 at 08:40