Head First C#: strange way to create a read-only property
Asked Answered
M

8

6

I am going through the Head First C# book and I can't figure out why they used the following way to create a property. It just seems inconsistent with the convention I'm seeing everywhere else and in the book itself too.

I understand that the pattern for creating properties is:

    private int myVar;

    public int MyProperty
    {
        get { return myVar; }
        set { myVar = value; }
    }

Based on the above pattern I would have written my code like this:

    private decimal cost;

    public decimal Cost
    {
        get
        {   
            cost = CalculateCostOfDecorations() + (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson) * NumberOfPeople; 
            if (HealthyOption)
            {
                cost *= .95M;
            }

            return cost;
        }

    }

In the book it is presented like the following:

    public decimal Cost
    {
        get
        {
            decimal totalCost = CalculateCostOfDecorations();
            totalCost += (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson)*NumberOfPeople;

            if (HealthyOption)
            {
                totalCost *= .95M;
            }
            return totalCost;
        }

    }

Both codes work just fine in the program. What is the best practice to create such properties? Is the decimal totalCost inside the property private? If so, why is it not declared before creating the property instead?

Also, what is the point of creating two lines of code:

            decimal totalCost = CalculateCostOfDecorations();
            totalCost += (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson)*NumberOfPeople;

when you can accomplish the exact same thing by writing:

cost = CalculateCostOfDecorations() + (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson) * NumberOfPeople;
Millymilman answered 14/8, 2015 at 20:34 Comment(2)
There is no reason to store cost as a backing field because it is recalculated each time the getter is called. The totalCost is a local variable, it doesn't exist outside the scope of the getter, its temporary for holding the calculation result until the value is returned. Using decimal with the same variable name twice is a syntax error and will give you an error when you try to compile.Lavalley
Thank you for your answer. Also, the "decimal" used twice was a typo, thanks for pointing it out.Millymilman
R
8

The primary difference between the two samples is, as you have noticed, that one has a backing field (which is what putting decimal cost outside the property definition does) and the other doesn't.

The totalCost variable in the second sample isn't a field at all (private or otherwise), it is simply a variable local to the get method.

Both are fine, though if you aren't using the backing field for anything, its not really necessary to have. As to your second question, I have no idea why they specifically did it that way, other than to make the variable declaration simpler.

As an aside, both samples are a bit off from standard C# practice, as thats an awful lot of logic for a property getter.

Reedy answered 14/8, 2015 at 20:40 Comment(0)
F
4

Both ways "work." If you're asking which way is best, I'd say neither.

Since Cost isn't really a field or property (it is something that is computed, and cannot be set by the caller), it would be more idiomatic to implement as a method which returns a value. No member variable should be needed.

public decimal GetCost()
{
    var cost = CalculateCostOfDecorations() + (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson) * NumberOfPeople; 
    if (HealthyOption)
    {
        cost *= .95M;
    }
    return cost;
}
Footpace answered 14/8, 2015 at 20:59 Comment(2)
Thanks! I wanted to ask whether it would be better to create a method instead of a property in this case, but that would have made the question even longer.Millymilman
I would definitely write a method here-- especially since it calls a method (CalculateCostOfDecorations()). Properties are supposed to be very lightweight and should not have side effects.Footpace
E
3

Is the decimal totalCost inside the property private? If so, why is it not declared before creating the property instead?

It is created inside the property to limit its scope. If you declared totalCost above the property, it would be accessible throughout the class itself.

Also, what is the point of creating two lines of code:

Usually, just for readability. One liners are great until you have to keep scrolling to see it in its entirety.

Echeverria answered 14/8, 2015 at 20:39 Comment(0)
G
2

As mentioned by others, neither of these is really idiomatic C#, however having the backing field for this could lead to errors later on in the code, e.g.:

private decimal cost;

public decimal Cost
{
    get
    {   
        cost = CalculateCostOfDecorations() + (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson) * NumberOfPeople; 
        if (HealthyOption)
        {
            cost *= .95M;
        }

        return cost;
    }

}

public decimal CalculateDiscountedCost() {
     return cost * 0.75m; //Note the deliberate mistake?
}

By (possibly accidentally) accessing the backing variable in the subsequent method, rather than the property you could easily develop some hard to maintain code. In this case, the discounted cost may be right when called, but would depend on the public propery Cost being accessed prior to calling the CalculateDiscountedCost method in order to set the backing variable.

As the backing variable is essentially unnecessary, it would be be better to do without it.

Goulden answered 14/8, 2015 at 20:47 Comment(2)
So by creating a variable local to the get method, as presented in the book, instead of the backing variable, this kind of error would be avoided. Do I understand correctly?Millymilman
That's correct. It's more about how maintainable your code is rather than any real difference in functionality.Goulden
I
1

The totalCost variable is not private, it's local variable inside of the getter. The variable does not exist outside of that method so it only takes up memory while the getting is running. Your private cost field will stay in memory as long as the class instance does. The variable is only ever used inside of the getter so it should be a local in the getter as the book shows. There is no reason for it to be a class field.

The point of creating two lines is so that it can fit a single line on the page. The code is exactly the same; it is just formatted differently.

Inexactitude answered 14/8, 2015 at 20:41 Comment(0)
L
0

yes, properties that are based on calculations do not require a private field to hold the value at all (unless you wish to cache the value to avoid recalculation on follow gets). in fact an even better (at least terser and easier to follow) way is to do it with no local variables at all:

public decimal Cost
{
    get
    {
        return (CalculateCostOfDecorations() +
               (CalculateCostOfBeveragesPerPerson() + CostOfFoodPerPerson)
                 * NumberOfPeople) * HealthyOption? .95m: 1m; 
     }
}
Logbook answered 14/8, 2015 at 20:40 Comment(0)
B
0

You should actually use the Get method to only return the value of the private variable, as this is the good practice to follow. It will make it easier for you to maintain other functions that may use the private variable 'just in case'.

Always use "setters" to change the value of the variable, "getters" to just return it without any changes.

Backwoodsman answered 14/8, 2015 at 20:44 Comment(1)
Can you provide an example, please?Millymilman
R
0

I would suggest that in cases like you have (where the get and set just return or set the value of the single backing variable) that you use "automatic" properties, such as this:

public int MyProperty { get; set; }

And, perhaps, you don't want external users of the class to have access to that setter:

public int MyProperty { get; private set; }
Rectangle answered 14/8, 2015 at 22:3 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.