Adam
Adam

Reputation: 1561

Open Closed Principle and Strategy Pattern Question

I have a couple of ideas but I wanted to see what the SO community would suggest.

I have an abstract class with an abstract Calculate method on it. I have 2 implementations of it that calculate differently. This screams Strategy pattern to me however one of the implementations requires that a selected_type variable be set because it is used inside the Calculate method. I want to follow the OCP so my Calculate method shouldn't take in the dependencies.

This class is retrieved from the DB via NHibernate and the selected_type variable won't be set until after the object has been created. I'm trying to avoid an if statement to set the selected_type only if it is of a specific implementation. What would be the best way?

Here is a code example:

public abstract class TagType
{
        public virtual long id { get; protected set; }
        public virtual string description { get; protected set; }

        protected TagType(){}

        protected TagType(string description)
        {
            this.description = description;
        }

        public abstract decimal Calculate();
}

public class TagTypeImpl1
{
    public virtual int tag_months { get; protected set; }

    protected TagType() { }

    protected TagType(string description, int tag_months): base(description)
    {
        this.tag_months = tag_months;
    }

    public override decimal Calculate()
    {
        return (12*tag_months);
    }
}

public class TagTypeImpl2
{
    public virtual int tag_months { get; protected set; }
    public virtual TagType selected_tag_type { get; protected set; }

    protected TagType() { }

    protected TagType(string description, int tag_months, TagType selected_tag_type): base(description)
    {
        this.tag_months = tag_months;
        this.selected_tag_type = selected_tag_type;
    }

    public override decimal Calculate()
    {
        return selected_tag_type.Calculate() + (12*tag_months);
    }
}

public class ConsumerController
{
    private readonly IRepository<TagType> repository;

    public ConsumerController(IRepository<TagType> repository)
    {
        this.repository = repository;
    }

    public ActionResult Index(long id)
    {
        var tag_type = repository.get(id);

        //If using TagTypeImpl2 then the selected_tag_type variable needs to be set
        //I want to avoid if(tag_type.GetType() == typeof(TagTypeImpl2)) set selected_tag_type
        var result = tag_type.Calculate();

        return Json(new {result});
    }
}

I might be trying to do too much with this class adn maybe the persisted entity class is the wrong place to have the Calculate method but it seemed the best place since it knows the most about how to do the calculation.

Upvotes: 1

Views: 694

Answers (2)

user348905
user348905

Reputation:

It's not the responsibility of your class to decide what the strategies need, it's the responsibility of the strategy. The whole idea is that you can call whatever strategy you're using the same way, all the time.

Simply make all strategies implement the same interface -including selected_type-, but one of them ignores the selected_type, the other uses it. It's up to the strategy itself to decide this.

Alternatively, your implementations of strategy can have more properties than are defined in the interface. If you can initialize the strategies from outside of your class, and it's not a problem for the initializing class to know more about the specific implementation you might be able to set the properties for only the specific strategy that needs it. The former solution is cleaner though (always using the same interface).

Upvotes: 0

BlueMonkMN
BlueMonkMN

Reputation: 25601

Would it make sense to create a virtual (overridable) function "Initialize" that one should call on all tag_type objects loaded from repository so they can do what was skipped by the default constructor that the parameterized constructor would have done?

Or can you change the default constructor to initialize selected_type to either the correct value or some value that will instruct the calculate method to correct it before using it?

Upvotes: 1

Related Questions