Design guidelines for avoiding changes in multiple classes

I am trying to figure out how to make a small application more elegant and more resilient to change.

Basically, this is a kind of project pricing calculator, and the problem is that there are many parameters that can affect pricing. I'm trying to avoid cluttering the code with lots of if-clauses for each parameter, but still I have, for example. if clauses in two places, checking the value of the size parameter.

I have a book "Head First Design Patterns" and I tried to find ideas there, but the closest I got was a decorator sample that has an example where the prices of Starbuzz coffee are set based on the seasonings added first and then later by adding a parameter size (Tall, Grande, Venti). But that didn't seem to help, because adding this parameter still seemed to add complexity to the if-clause in many places (and this was an exercise they didn't explain further).

What I'm trying to avoid is to change multiple classes if the parameter were to change, or add a new parameter, or at least change as few places as possible (there's some fancy design principle word for this, t rememeber: -)).

Below is the code. Basically it calculates a price for a project that has Writing and Analyzing tasks with a size parameter and different price models. Later on, other options will appear, such as "How's the new product?" (New, 1-5 years old, 6-10 years old) etc. Any advice on the best design would be much appreciated, be it a "design pattern" or just good object-oriented principles that make it resilient to change (like adding another size or changing one of the size values ​​and only changing in one place, not in multiple if clauses):

public class Project
{
    private readonly int _numberOfProducts;
    protected Size _size;
    public Task Analysis { get; set; }
    public Task Writing { get; set; }

    public Project(int numberOfProducts)
    {
        _numberOfProducts = numberOfProducts;
        _size = GetSize();
        Analysis = new AnalysisTask(numberOfProducts, _size);
        Writing = new WritingTask(numberOfProducts, _size);

    }

    private Size GetSize()
    {
        if (_numberOfProducts <= 2)
            return Size.small;
        if (_numberOfProducts <= 8)
            return Size.medium;
        return Size.large;
    }
    public double GetPrice()
    {
        return Analysis.GetPrice() + Writing.GetPrice();
    }
}

public abstract class Task
{
    protected readonly int _numberOfProducts;
    protected Size _size;
    protected double _pricePerHour;
    protected Dictionary<Size, int> _hours;
    public abstract int TotalHours { get; }

    public double Price { get; set; }

    protected Task(int numberOfProducts, Size size)
    {
        _numberOfProducts = numberOfProducts;
        _size = size;
    }

    public double GetPrice()
    {
        return _pricePerHour * TotalHours;
    }
}

public class AnalysisTask : Task
{
    public AnalysisTask(int numberOfProducts, Size size)
        : base(numberOfProducts, size)
    {
        _pricePerHour = 850;
        _hours = new Dictionary<Size, int>() { { Size.small, 56 }, { Size.medium, 104 }, { Size.large, 200 } };
    }

    public override int TotalHours
    {
        get { return _hours[_size]; }
    }
}

public class WritingTask : Task
{
    public WritingTask(int numberOfProducts, Size size)
        : base(numberOfProducts, size)
    {
        _pricePerHour = 650;
        _hours = new Dictionary<Size, int>() { { Size.small, 125 }, { Size.medium, 100 }, { Size.large, 60 } };
    }

    public override int TotalHours
    {
        get
        {
            if (_size == Size.small)
                return _hours[_size] * _numberOfProducts;
            if (_size == Size.medium)
                return (_hours[Size.small] * 2) + (_hours[Size.medium] * (_numberOfProducts - 2));
            return (_hours[Size.small] * 2) + (_hours[Size.medium] * (8 - 2)) + (_hours[Size.large] * (_numberOfProducts - 8));
        }
    }
}

public enum Size
{
    small, medium, large
}

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();
        List<int> quantities = new List<int>();

        for (int i = 0; i < 100; i++)
        {
            quantities.Add(i);
        }
        comboBoxNumberOfProducts.DataSource = quantities;
    }

    private void comboBoxNumberOfProducts_SelectedIndexChanged(object sender, EventArgs e)
    {
        Project project = new Project((int)comboBoxNumberOfProducts.SelectedItem);
        labelPrice.Text = project.GetPrice().ToString();
        labelWriterHours.Text = project.Writing.TotalHours.ToString();
        labelAnalysisHours.Text = project.Analysis.TotalHours.ToString();
    }
}

      

At the end, this is a simple running code to call in a change event for the combobox that set the size ... (By the way, I don't like the fact that I have to use multiple dots to get to TotalHours at the end here, as far as I remember it breaks " the principle of least knowledge "or" the law of demeter ", so the contribution to this will also be appreciated, but this is not the main question)

Hello,

Anders

+2


a source to share


4 answers


First of all, you should, in my opinion, rethink the design. Projects don't look like this, and as far as I understood in your code, there is no way for you to add more tasks to the project. Also consider separating the project and the way in which you will calculate the prize. What if you have different calculation methods? This is also about responsibility, soon you may grow and it will be difficult to separate the way of calculating the price and the structure of the project. It is common to avoid "if" using polymorphism - perhaps you would like to have different types of projects depending on their parameters. This can be achieved with a Factory method that will take arguments, do the "if" s once, and create some subtype of Project that will know how to calculate its payoff correctly. If you separate project and calculation,than consider instead a strategy template for calculating winnings. The concern about the demeter law is adequate here because you are exposing tasks. Try using a method instead that will return the total price and will delegate. The reason is that this class, where this method (project or calculation strategy) will be, can decide how to calculate it, it can also receive information from other tasks. You will need to tweak this method if you plan to add more tasks, perhaps using one method with a string or enum parameter to select a specific task for calculating payoff. BTW. why are you stressing so much?which will return the total price and will delegate. The reason is that this class, where this method (project or calculation strategy) will be, can decide how to calculate it, it can also receive information from other tasks. You will need to tweak this method if you plan to add more tasks, perhaps using one method with a string or enum parameter to select a specific task for calculating payoff. BTW. why are you stressing so much?which will return the total price and will delegate. The reason is that this class, where this method (project or calculation strategy) will be, can decide how to calculate it, it can also receive information from other tasks. You will need to tweak this method if you plan to add more tasks, perhaps using a single method with a string or enum parameter to select a specific task for calculating payoff. BTW. why are you stressing so much?to select a specific problem for calculating the winnings. BTW. why are you stressing so much?to select a specific problem for calculating the winnings. BTW. why are you stressing so much?



+3


a source


If you have an if ... else statement like this based on the properties of a class, try to abandon it with a strategy template. You can try a book called "Refactor to Patterns" which is a good book on refactoring.



+1


a source


So the app you developed has what I would say is one major design gap:

It accepts one set of usage data.

By this I mean that it assumes that there are only two possible tasks: each has a hard-coded price (something that simply doesn't exist in the business world), and each task computes a "clock" deterministically against a constant set of sizes. My recommendation is to make almost everything customizable either with a database to store new possible tasks / properties / prices / hourly ratios / sizes or some other storage medium and write a configuration form to manage it.

This will almost immediately remove the design problem you mean, because you remove all hardcoded domain contexts and instead set a config use case that can then be exposed via the API if someone doesn't like your setup method or you want to use it whichever -or another way.

Edit: I wanted to comment on this below, but left the room:

expand the depth of your xml to meaningfully represent larger data structures (WritingTask and AnalysisTask) as well as their constituent parts (properties and methods). These methods can often be defined by a set of rules. You can tokenize properties and rules so they can interact independently. Example:

<task name="WritingTask">
<property name="numberofproducts" type="int"/>
<property name="Size" type="size">
    <property name="Price" type="decimal">
    <param name="priceperhour" value="650">
    </property>
<property name="hours" type="Dictionary">
    <param name="Size.small" value="125"/> 
    <param name="Size.medium" value="100"/>     
        <param name="Size.large" value="60"/>   
    </property>     
    <method name="TotalHours">
        <rule condition="_size == Size.Small">
    <return value="_hours[_size] * _numberofproducts"/>
        </rule>
        <rule condition="_size == Size.medium">
        <return value="(_hours[Size.small] * 2) + (_hours[Size.medium] * _numberOfProducts - 2))"/>
    </rule>
    <return value="(_hours[Size.small] * 2) + (_hours[Size.medium] * (8 - 2)) + (_hours[Size.large] * (_numberOfProducts - 8))"/> 
    </method> 
</task>

      

Anyway, it's too late in the morning for me to try this any further, but tomorrow I'll be watching over you. By setting the resulting properties and binding the methods in the config to the rules, you leave the if negotiation in your dataset (it should know best) and tweak your code to interpret that data accurately. It remains flexible enough to handle growth (via the sudo language to create a dataset), but remains unchanged internally, without the need to improve on more versatile performance.

0


a source


It uses underscores for member variables. you can use "I". or that. "instead, but it's just as clear. I believe it comes from some old java style standard? I personally love that a lot.

0


a source







All Articles