CODEWITHSUNDEEP
c#coding-stylecode-structure
Bart Calixto
Bart Calixto

Reputation: 19705

Getter, Function or just set it on constructor?

I can't find a good performant and legibility way to write some computed inside a class. Imagine the following class and all ways to get the FinalPrice:

public class Order {
    public Order(Product[] products) {
        Items = products;

option 1: Having a variable declaration for each property that want to be computed, horrible legibility

        var orderPrice = products.Sum(p => p.Price * p.Quantity);
        var orderTaxes = products.Sum(p => p.Taxes * p.Quantity);
        var orderDiscount = products.Sum(p => p.Price * p.Quantity * p.Discount);

        OrderPrice = orderPrice;
        OrderTaxes = orderTaxes;
        OrderDiscount = orderDiscount;

        FinalPrice = orderPrice + orderTaxes - orderDiscount;

option 2: having the problem of the order in the class matters! FinalPrice line can't be before the others or it won't work but won't throw error.

        OrderPrice = products.Sum(p => p.Price * p.Quantity);
        OrderTaxes = products.Sum(p => p.Taxes * p.Quantity);
        OrderDiscount = products.Sum(p=> p.Price * p.Quantity * p.Discount);

        FinalPrice = OrderPrice + OrderTaxes - OrderDiscount;

option 3: rewriting all the formulas - Bad for manteinance. Most likely to instroduce differences in prices later on.

        FinalPrice = products.Sum(p => p.Price * p.Quantity) + 
                     products.Sum(p => p.Taxes * p.Quantity) - 
                     products.Sum(p => p.Price * p.Quantity * p.Discount);

    }

option 4: using getters. This will be calculated everytime it's called. This is a simple calculation, but assume something more code heavily.

    public decimal FinalPrice { get {
        return OrderPrice + OrderTaxes - OrderDiscount;
    } }
}

option 5: using a function. Is this a good or bad thing ??

    public decimal CalculateFinalPrice() {
        return OrderPrice + OrderTaxes - OrderDiscount;
    }

Upvotes: 2

Views: 532

Answers (2)

Alex Siepman
Alex Siepman

Reputation: 2598

I would do all the logic in the getters:

public decimal CalculateFinalPrice
{
    get { return CalculateOrderPrice + CalculateOrderTaxes - CalculateOrderDiscount; }
}

public decimal CalculateOrderPrice
{
    get { return products.Sum(p => p.Price*p.Quantity); }
}

public decimal CalculateOrderTaxes
{
    get { return products.Sum(p => p.Taxes*p.Quantity); }
}

public decimal CalculateOrderDiscount
{
    get { return products.Sum(p => p.Price*p.Quantity*p.Discount); }
}

If this is slow in your secenario, you can cache the properties. 

private decimal? _calculateOrderPrice;
public decimal CalculateOrderPrice
{
    get
    {
        if (_calculateOrderPrice == null)
        {
            _calculateOrderPrice = products.Sum(p => p.Price*p.Quantity;
        }
        return _calculateOrderPrice.Value;
    }
}

If you go to the definition of the property, you can see immediately how it is calculated. Also you don't care about wich calculation needs to be done first.

Upvotes: 4

Karl Anderson
Karl Anderson

Reputation: 34846

I would create methods for CalculateFinalPrice, CalculateOrderPrice, CalculateOrderTaxes and CalculateOrderDiscount, like this:

public decimal CalculateFinalPrice() {
    return CalculateOrderPrice() + CalculateOrderTaxes() - CalculateOrderDiscount();
}

public decimal CalculateOrderPrice()
{
    // Logic here to calculate order price
    return theOrderPrice;
}

public decimal CalculateOrderTaxes()
{
    // Logic here to calculate order taxes
    return theOrderTaxes;
}

public decimal CalculateOrderDiscount()
{
    // Logic here to calcuate order discount
    return theOrderDiscount;
}

This provides you with more, but smaller pieces that are easier to maintain, read and unit test, because each method has a single responsibility.

Upvotes: 4

Related Questions