Creational Patterns suggestion

Question

I'm struggling to find a good way to handle a problem that seems to be nice.

My entire model relies on the concept of pricing strategies. These strategies calculate a specific price based on rules. Rules just how to calculate the price for a given time range (8AM to 12AM cost you that amount of money...)

I currently have 6 strategies, each of them inheriting from RuledBasedPricingStrategy which itself implements IStrategy

public abstract class RuledBasedPricingStrategy : IPricingStrategy
{
    public abstract string FriendlyName { get; }
    protected abstract ICollection<IPricingRule> Rules { get; }

    protected Booking Booking { get; }
    protected Resource Resource { get; }
    protected TecTacClient Client { get; }

    protected RuledBasedPricingStrategy(Booking booking, Resource resource, TecTacClient client)
    { 
       // Just checking and assigning values(...) 
    }

    public abstract bool IsEligible();

    public Pricing Build()
    {
        var strategy = new Pricing(FriendlyName, Resource.CurrencyCode);
        foreach (var r in Rules.OrderByDescending(x => x.Priority))
            r.Apply(strategy, Booking, Resource, Client);
        return strategy;
    }
}

public class RegularPricingCalculatorFactory : RuledBasedPricingStrategy
{
    public override string FriendlyName => "Regular Pricing";

    protected override ICollection<IPricingRule> Rules => new List<IPricingRule>
    {
        new OfficeHourPricingRule(),
        // Non Office Hours
        new OnePercentSurchagePricingRule()
    };

    public override bool IsEligible() => (...)

    public RegularPricingCalculatorFactory(Booking booking, Resource resource, TecTacClient client)
        : base(booking, resource, client)
    { }
}

public class HalfdayPricingStrategy : RuledBasedPricingStrategy
{
    public override string FriendlyName => "Half day Pricing";

    protected override ICollection<IPricingRule> Rules => new List<IPricingRule>
    {
        new HalfDayPricingRule(),
        new OfficeHourPricingRule(),
        // Non Office Hours
    };

    public override bool IsEligible() => (...)

    public HalfdayPricingStrategy(Booking booking, Resource resource, TecTacClient client) 
        : base(booking, resource, client) { }
}

IRule is a simple interface. Rules accepts a pricing as a parameter and will add as many pricing steps as necessary:

public interface IPricingRule
{
    int Priority { get; }
    void Apply(Pricing pricing, Booking booking, Resource resource, TecTacClient client);
}

The pricing is a simple class as shown below:

public class Pricing
{
    public string Name { get; }
    public string CurrencyCode { get; }
    private readonly List<PricingStep> _steps;
    public ICollection<PricingStep> Steps => _steps.AsReadOnly();
    public decimal TotalPrice => Steps.Sum(x => x.Price);
 }

And so on (...)

I'm looking for a good way to instantiate all these strategies. As you noticed they take the booking, resource and client objects as ctor arguments. My initial point was to avoid passing these argument twice (IsEligible and Build). However then I need to build a "factory" that will know all strategy types and instantiate them every time I get a new request:

    public IEnumerable<IPricingStrategy> Find(Booking booking, Resource resource, TecTacClient client)
        => _strategiesType
            .Select(t => Activator.CreateInstance(t, booking, resource, client))
            .OfType<IPricingStrategy>();

It does not seem to be the right approach. Strategy should be instantiated once and then reused to calculate strategy. However I can't in that case I will end up doing something like

foreach(var s in strategies)
    if (s.IsEligible(booking, resource, client))
        yield return s.Build(booking, resource, client);

What patterns could I use to simplify/clean this code?

Thx Seb

Iain Galloway · Accepted Answer · 2017-11-10T09:38:34.033

As you've noted, passing your parameters into your constructor means that you'll need to make a new instance of each of your IPricingStrategies every time one of those parameters changes.

Yet, you don't want to pass in the same set of parameters to the two methods of your Strategy.

Is there a good reason those two methods are separate in the first place? It seems to me that the caller is never going to want to call IsEligible except to decide whether or not to call Build. We could move that decision into the Strategy, and return the (composite) result:-

// You can make this guy more sophisticated by building a proper
// option type if you like, but this is good enough as an example.
class PricingStrategyResult
{
  public bool IsEligible { get; set; }
  public Pricing Pricing { get; set; }
}

interface IPricingStrategy
{
  // Your caller passes the parameters and receives both values
  // at once.
  PricingStrategyResult Build(
    Booking booking,
    Resource resource,
    TecTacClient client)
}

// But you can still split the logic up at the implementation
// level if you need to.
public abstract class RuledBasedPricingStrategy : IPricingStrategy
{
  protected abstract bool IsEligible();

  public PricingStrategyResult Build(
    Booking booking,
    Resource resource,
    TecTacClient client)
  {
    if (!this.IsEligible)
    {
      return new PricingStrategyResult()
      {
        IsEligible = false
      };
    }

    var pricing = new Pricing(FriendlyName, Resource.CurrencyCode);

    foreach (var r in Rules.OrderByDescending(x => x.Priority))
    {
      r.Apply(pricing, booking, resource, client);
    }

    return new PricingStrategyResult()
    {
      IsEligible = true,
      Pricing = pricing
    };
  }
}

Then you call it like:-

results = strategies.Build(booking, resource, client)
                    .Where(x => x.IsEligible)
                    .Select(x => x.Pricing);

Creational Patterns suggestion

1 Answers1