How to handle same code parts in Factory Method?

Question

I have classes:

public abstract class House{
   public string Name {set;get;}
   public SomeClass Property1 {set;get;}
   public OtherClass Property2 {set;get;}
}

public class WoodenHouse:House{
   public string WoodType {set;get;}
   public int WoodAge {set;get;}
}

public class StoneHouse:House{
  public string StoneType{set;get;}
}

And trying to create Factory Method patthern for this:

    abstract class Creator
    {
        public abstract HouseInfo Info { get; set; }

        public Creator()
        {
        }

        public abstract House FactoryMethod();
    }

    class WoodenHouseCreator : Creator
    {
        public override HouseInfo Info { get; set; }

        public WoodenHouseCreator(WoodenHouseInfo info)
        {
            Info = info;
        }

        public override House FactoryMethod()
        {
            var info = Info as WoodenHouseInfo;

            var woodenHouse = new WoodenHouse();
            woodenHouse.Name = info.Name;
            woodenHouse.Floors = info.Floors;
            woodenHouse.RoofType = info.RoofType;
            woodenHouse.WoodType = info.WoodType;
            woodenHouse.WoodAge = info.WoodAge;
            return woodenHouse;
        }
    }

    class StoneHouseCreator : Creator
    {
        public override HouseInfo Info { get; set; }

        public StoneHouseCreator(StoneHouseInfo info)
        {
            Info = info;
        }

        public override House FactoryMethod()
        {
            var info = Info as StoneHouseInfo;

            var stoneHouse = new StoneHouse();
            stoneHouse.Name = info.Name;
            stoneHouse.Floors = info.Floors;
            stoneHouse.RoofType = info.RoofType;
            stoneHouse.StoneType = info.StoneType;
            return stoneHouse;
        }
    }

Here is classes what contains information to create house:

    class HouseInfo
    {
        public string Name { set; get; }
        public int Floors { set; get; }
        public string RoofType { set; get; }
    }

    class WoodenHouseInfo : HouseInfo
    {
        public string WoodType { set; get; }
        public int WoodAge { set; get; }
    }

    class StoneHouseInfo : HouseInfo
    {
        public string StoneType { set; get; }
    }

And Usage:

        var houseInfo = new WoodenHouseInfo{
            Name = "HouseName",
            Floors = 2,
            RoofType = "Triangle",
            WoodType = "Pine",
            WoodAge = 100
        };

        House house;

        if(houseInfo is WoodenHouseInfo)
        {
            var creator = new WoodenHouseCreator(houseInfo);
            house = creator.FactoryMethod();
            Console.Write((house as WoodenHouse).WoodAge);
        }

Full code fiddle.

My problem that how to handle code duplication. I mean here is a lot of lines what fills base House object properties. How can i write that code only once?
Or i should not to use Factory Method?

Add Populator Class

class HousePopulator
    {
        public void PopulateHouse(HouseInfo info, House house)
        {
            house.Name = info.Name;
            house.Floors = info.Floors;
            house.RoofType = info.RoofType;
        }
    }

And usage:

abstract class Creator
{
    public abstract HouseInfo Info{get;set;}
    public HousePopulator HousePopulator {get;set;}
    public Creator()
    {
        HousePopulator = new HousePopulator();
    }
    public abstract House FactoryMethod();
}

class WoodenHouseCreator : Creator
{
    public override HouseInfo Info{get;set;}

    public WoodenHouseCreator(WoodenHouseInfo info)
    {
        Info = info;
    }
    public override House FactoryMethod()
    {
        var info = Info as WoodenHouseInfo;

        var woodenHouse = new WoodenHouse();
        HousePopulator.PopulateHouse(Info, woodenHouse);
        woodenHouse.WoodType = info.WoodType;
        woodenHouse.WoodAge = info.WoodAge;
        return woodenHouse;
    }
}

The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. — BCdotWEB
– BCdotWEB, Commented Feb 6, 2020 at 16:18
Do not update your question with insights from answers. This makes it very confusing And Is against rules of this site. If you want to review your modifications, post another question. — slepic
– slepic, Commented Feb 6, 2020 at 18:06
Also please add more context. In current state your code Is very hard to review as Its purpose remains hidden or represents a generic problem which would be off topic here. Your code does not even compile. — slepic
– slepic, Commented Feb 6, 2020 at 18:10
FYI this is more abstract factory than factory method, and I believe you will want to look into the builder pattern for this kind of thing. — Mathieu Guindon
– Mathieu Guindon, Commented Feb 7, 2020 at 18:11

Philip Atz · Accepted Answer · 2020-02-06 13:56:57Z

Currently your factories instantiate the new objects and then fill in all of their properties with the right values. You could split instantiation from property value assignment. Your StoneHouseCreator could instantiate a StoneHouse, use a HousePopulator that populates the values that all objects of type House have in common, and then the StoneHouseCreator could populate the rest of the values that are exclusive to a StoneHouse. That same HousePopulator could also be used by your WoodenHouseCreator, which would then proceed to populate the WoodenHouse-specific properties.

Actually, there are two ways of doing this. Either you keep it as it was originally (i.e. using inheritance), give your base class a PopulatorMethod(...) that populates the properties of a House, and call base.PopulatorMethod(...) from your override of PopulatorMethod(...) in the child classes. Or you drop the inheritance between your factories completely, you make your FactoryMethod() implementations accept instances of WoodenHouseInfo or StoneHouseInfo depending on the implementing class, and have them use a HousePopulator as you have done.

If you go down the second route, you don't need an abstract parent class, although an interface would be nice, and you can move the HousePopulator = new HousePopulator() assignment to the individual factory classes.

If you want to philosophise about this at a higher level, these are the problems that we run into because of inheritance. Factories, that is the logical separation of object use from object creation, are more naturally suited to cases where you use composition over inheritance. If you are interested more in this, I would recommend reading this excellent article on the topic.

I update question with Populator class(also update fiddler). Do you mean solution like this? — Kliver Max
– Kliver Max, Commented Feb 6, 2020 at 9:50
There are two ways of doing this. Either you keep it as it was originally (i.e. using inheritance), give your base class a PopulatorMethod(...) that populates the properties of a House, and call base.PopulatorMethod(...) from your override of PopulatorMethod(...) in the child classes. Or you drop the inheritance between your factories completely, you make your FactoryMethod() implementations accept instances of WoodenHouseInfo or StoneHouseInfo depending on the implementing class, and have them use a HousePopulator as you have done. — Philip Atz
– Philip Atz, Commented Feb 6, 2020 at 10:10
If you go down the second route, you don't need an abstract parent class, although an interface would be nice, and you can move the HousePopulator = new HousePopulator(); assignment to the individual factory classes. — Philip Atz
– Philip Atz, Commented Feb 6, 2020 at 10:11

iSR5 · Accepted Answer · 2020-02-07 04:46:25Z

You need to simplify your class to its standard form, which is House needs Name and Type, then go from there.

Take it this way, if you need to create a new house, then you can write pseudo code that want something like :

var house = new House
{
    Name = "House Name",
    Type = new WoodenHouse
    {
        WoodType = "Pine",
        WoodAge = 100,
        Floors = 2,
        RoofType = "Triangle"
    }
};

Since Floors and RoofType are releated to the House, then you can include them into the House instead.

Example :

var house = new House
{
    Name = "House Name",
    Type = new WoodenHouse
    {
        WoodType = "Pine",
        WoodAge = 100
    },
    Floors = 2,
    RoofType = "Triangle"
};

then we can implement this :

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

    public IHouseType Type { get; set; }

    public int Floors { get; set; }

    public string RoofType { get; set; }
}

public interface IHouseType 
{
    // define your contract
     string TypeName { set; get; }
}

public class WoodenHouse : IHouseType
{
    public string TypeName { set; get; }

    public int WoodAge { set; get; }

}

public class StoneHouse : IHouseType
{
    public string TypeName { set; get; }
}

and your Creator class can be something like this :

public class HouseCreator
{
    private House _house;

    public HouseCreator(House house) { house = _house; }


    public House FactoryMethod()
    {
        if(_house.Type is null) { throw new ArgumentNullException(); } // it's not defined

        /*
         NOTE : 
            Floors & RoofType are shared properties, so it can be processed outside the following if blocks
         */
        var floors   = _house.Floors;
        var RoofType = _house.RoofType;


        if (_house.Type is WoodenHouse)
        {
            // process the WoodenHouse logic
        }

        if(_house.Type is StoneHouse)
        {
            // process the StoneHouse logic and save it 

        }
        return _house;// return the processed house object.
    }
}

It's straight forward process, and there is no need to use duplicated objects like HouseInfo, and the Creator and HousePopulator in current context, I don't see any usage of them, since new keyword does that for you. What you need might be to implement FactoryMethod() and PopulateHouse() inside House class, which would make more sense to me at least. The factory method would act as a handler where you process the logic to your object internally, and then pass it to the Populate method where you execute the output publicly.

So, the conversion usage might be simplified to something like this :

// Create a new house
var house = new House
{
    Name = "House Name",
    Type = new WoodenHouse
    {
        TypeName = "Pine",
        WoodAge = 100
    },
    Floors = 2,
    RoofType = "Triangle"
};


//Populate It 
house.Populate();

Another thing that might be also useful, using Enum in RoofType would me more appropriate than string. so this line :

RoofType = "Triangle"

can be converted to this :

RoofType = HouseRoofType.Triangle

Stack Exchange Network

How to handle same code parts in Factory Method?

2 Answers 2

You must log in to answer this question.

Hot Network Questions

How to handle same code parts in Factory Method?

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions