How to combine these fill and create methods or utilize generics in factory?

Question

Domain object "Contragent"

Let's say I have an hierarchy of classes:

public class BaseContragent { public int Id { get; set; } } public class PersonContragent : BaseContragent { public string FirstName { get; set; } public string LastName { get; set; } } public class CompanyContragent : BaseContragent { public string CompanyName { get; set; } } public class BankCompanyContragent : CompanyContragent { public string BankBic { get; set; } } public class LocalBankCompanyContragent : BankCompanyContragent { public string NationalBankIdentificator { get; set; } }

Actually, this hierarchy has many classes, it has much of business logic, and all fields are taken from business domain logic, so I can't change it just because of inconvenient external service.

Contragent - PersonContragent -- PersonWithoutCitizenshipContragent -- CitizenshipContragent --- LocalCitizenshipContragent --- ForeignCitizenshipContragnet - CompanyContragent -- BankCompanyContragent --- LocalBankCompanyContragent etc.

Now, external system sends requests to create object

I have a request to create an object from external system, which looks like this:

public class CreateRequest { public int Id { get; set; } public string Name { get; set; } // default for base, value for subclasses public int Type { get; set; } // 0 - for Base, 1 for Person, 2 for Company // many other fields belonging to different types }

There is only one type of create request for all types of Contragent, and which one should I use depends on Type value.

In order to isolate my system from this strange external system I have implemented some sort of anti-corruption layer with factories which create objects of this hierarchy:

public abstract class BaseContragentFactory { protected abstract Contragent Create(); protected abstract Fill(Contragent ca, CreateRequest createRequest); public Base CreateAndFill(CreateRequest createRequest) { var ca = Create(); Fill(ca, createRequest); return ca; } } public class SimpleContragentFactory : BaseContragentFactory { protected virtual Contragent Create() { return new Contragent(); } protected virtual void Fill(Contragent ca, CreateRequest createRequest) { ca.Id = createRequest.Id; } } public class PersonContragentFactory : SimpleContragentFactory { protected override Contragent Create() { return new PersonContragent(); } protected override void Fill(Contragent ca, CreateRequest createRequest) { var pca = ca as PersonContragent; if (pca == null) throw new InvalidOperationException("..."); base.Fill(pca, createRequest); string[] nameParts = createRequest.Name.Split(";"); // Firstname;Lastname pca.FirstName = nameParts[0]; pca.LastName = nameParts[1]; } } public class CompanyContragentFactory : SimpleContragentFactory { protected override Contragent Create() { return new CompanyContragent(); } protected override void Fill(Contragent ca, CreateRequest createRequest) { var bca = ca as CompanyContragent; if (bca == null) throw new InvalidOperationException("..."); base.Fill(bca, createRequest); bca.CompanyName = createRequest.Name; } }

I have such factories for almost every type of Contragent.

public void Create(CreateRequest request) { BaseContragentFactory factory; switch (request.Type) { case 0: factory = new SimpleContragentFactory(); break; case 1: factory = new PersonContragentFactory(); break; case 2: factory = new CompanyContragentFactory(); break; default: throw new InvalidOperationException("..."); } Contragent ca = factory.CreateAndFill(request); }

However, I don't like that this codes has casts, checks and that type-safety is only maintained by a developer.

So, I have two question about improving code type-safety and readability:

Question 1: how to combine Create and Fill methods? How to make this class have only one method. The problem that I need to be able to create document only once at the top-inherited class and then call base fill methods.

Question 2: is there any way to utilize generics to make these factories more type-safe and accept generic-typed values? May be, s1omething like this:

public abstract class BaseContragentFactory<T> where TContragent : Contragent, new() { protected TContragent Create() { return new TContragent(); } // ...

Why are Create and Fill separate methods? They should be single method. That way, most of the type casts disappear. — Euphoric
– Euphoric, Commented Mar 24, 2017 at 5:59
@Euphoric that's exactly what question is about :) How to make it a single method? — Yeldar Kurmangaliyev
– Yeldar Kurmangaliyev, Commented Mar 24, 2017 at 6:08
Uhhh .. just join those two methods together? It is so simple your question doesn't make much sense. Where is the problem? — Euphoric
– Euphoric, Commented Mar 24, 2017 at 6:09
@Euphoric How will it look like? Could you please provide the code? If it was a simple "combine two methods" problem, I wouldn't ask it :) — Yeldar Kurmangaliyev
– Yeldar Kurmangaliyev, Commented Mar 24, 2017 at 6:11
I see now. The Id property on the base type is what complicates things. — Euphoric
– Euphoric, Commented Mar 24, 2017 at 6:19

Euphoric · Accepted Answer · 2017-03-24 07:05:49Z

Based on your updated question, I would suggest design below:

public class BaseContragent { public int Id { get; set; } } public class PersonContragent : BaseContragent { public string FirstName { get; set; } public string LastName { get; set; } } public class CompanyContragent : BaseContragent { public string CompanyName { get; set; } } public class CreateRequest { public int Id { get; set; } public string Name { get; set; } // default for base, value for subclasses public int Type { get; set; } // 0 - for Base, 1 for Person, 2 for Company } public interface IContragentFactory { BaseContragent Create(CreateRequest createRequest); } public class SimpleContragentFactory : IContragentFactory { public BaseContragent Create(CreateRequest createRequest) { var pca = new BaseContragent(); Fill(pca, createRequest); return pca; } public static void Fill(BaseContragent ca, CreateRequest createRequest) { ca.Id = createRequest.Id; } } public class PersonContragentFactory : IContragentFactory { public BaseContragent Create(CreateRequest createRequest) { var pca = new PersonContragent(); Fill(pca, createRequest); return pca; } public static void Fill(PersonContragent pca, CreateRequest createRequest) { SimpleContragentFactory.Fill(pca, createRequest); string[] nameParts = createRequest.Name.Split(';'); // Firstname;Lastname pca.FirstName = nameParts[0]; pca.LastName = nameParts[1]; } } public class CompanyContragentFactory : IContragentFactory { public BaseContragent Create(CreateRequest createRequest) { var bca = new CompanyContragent(); Fill(bca, createRequest); return bca; } public static void Fill(CompanyContragent bca, CreateRequest createRequest) { SimpleContragentFactory.Fill(bca, createRequest); bca.CompanyName = createRequest.Name; } } public class FactoryClient { public void Create(CreateRequest request) { IContragentFactory factory; switch (request.Type) { case 0: factory = new SimpleContragentFactory(); break; case 1: factory = new PersonContragentFactory(); break; case 2: factory = new CompanyContragentFactory(); break; default: throw new InvalidOperationException("..."); } BaseContragent ca = factory.Create(request); } }

It gets rid of inheritance and uses "composition" using static Fill method. It then is on developer to call proper parent factory Fill method, but there are not type casts.

Ewan · Accepted Answer · 2017-03-24 07:23:42Z

Generics don't really help in this situation, as all your classes inherit from the same base class.

However, I would suggest that you have too many classes.

You are attempting to get around the conditional, 'if i want type x construct it like this' But that is impossible because you have different construction parameters.

If you refactor you code to put the construction in a constructor, requiring the appropriate CreateRequest and put a switch case on the Type of CreateRequest in the factory method your code will be considerably shorter and simpler

Alternatively you can try using a DI container and configure it via xml config to mimic your factory.

This will give you your 'no code change' style solution, However, it will just be doing the same thing by reflection under the hood. ie examine the incoming CreateRequest type, parse the logic from the xml and instanciate the mapped object via the constuctor.

since everyone is doing code example i will attempt one on my phone.

public class Person : BaseContragrent { public Person(CreateRequestPerson cr) : base(cr) { this.FirstName = cr.FirstName; ..... } } public class Factory { public BaseContragent Create(CreateRequest cr) { if(cr is CreateRequestPerson) { return new Person(cr); } ..... } }

I would be tempted to go even further and lose the CreateRequests and just pass in more parameters and a Type enum.

Stack Exchange Network

How to combine these fill and create methods or utilize generics in factory?

Domain object "Contragent"

Now, external system sends requests to create object

2 Answers 2

Hot Network Questions

How to combine these fill and create methods or utilize generics in factory?

Domain object "Contragent"

Now, external system sends requests to create object

2 Answers 2

Related

Hot Network Questions