Chronicide
Chronicide

Reputation: 1152

Is there some way to define getters/setters of a private property without having a second property to hold the value?

I have a public readonly property that calculates its value based upon a couple of other private variables. I read from the calculated property a lot. The variables that are used in the calculation change very rarely. I currently have the getter of the readonly perform the calculation logic, but it makes more sense to me to push the new value into the property when the dependant variables change.

An example of my current code:

public class rectangle()
{
    public rectangle(int height, int width)
    {
        _height = height;
        _width = width;
    }

    public void resize(int heightOffset, int widthOffset)
    {
        _height += heightOffset;
        _width += widthOffset;
    }

    private int _height;
    private int _width;
    public int area
    {
        get
        {
            return _height * _width;
        }
    }
}

The above code is overly simplified, but does a good job of showing what I mean. _height and _width can change, but they rarely ever do. The area property is read from a lot... I'd estimate that the _height/_width change once for every 100,000+ reads of the area property.

Now, if it was just multiplication, I wouldn't be concerned... but the actual calcuations are complex and I'm seeing a lot of run time being sunk into repetitous superfluous calculations.

I know that I can do the following to fix it:

...

private int _height;
private int height
{
    set
    {
        _height = value;
        _area = _height * _width;
    }
}

private int _width;
private int width
{
    set
    {
        _width = value;
        _area = _height * _width;
    }
}

private int _area;
public int area
{
    get
    {
        return _area;
    }
}

...

The above code makes a lot more sense given my circumstances, but it just seems to me that there must be a better way to do this. I know that, given my first example, I could get rid of the properties and move the calculation logic to the resize() method... but in the real code it isn't just one method that affects the private values. I don't want to have to comb through my code to find every occurrence, and it doesn't seem maintainable to do so.

Is the second method the way to go? I know it works, but something about it feels wrong. I think my concern is that it seems odd to have a private property storing it's value into a private variable. I may be overthinking this.

Upvotes: 3

Views: 410

Answers (5)

Chris Pfohl
Chris Pfohl

Reputation: 19044

Note: I will use traditional C# coding style (I.e.: caps for class names and public properties) for ease of reading. I recommend you do the same

One way to fix your problem is to simply make the Rectangle class immutable and use Lazy<T> for the area to delay calculating your expensive thing:

public class Rectangle
{
    private readonly Lazy<int> _area;

    public Rectangle(int height, int width)
    {
        Height = height;
        Width = width;
        _area = new Lazy<int>(() => Height * Width);
    }

    public int Height { get; private set; }

    public int Width { get; private set; }

    public int Area
    {
        get
        {
            return _area.Value;
        }
    }
}

The benefits of immutable classes/structs are countless. The downside is that whenever you want to 'change' a Rectangle object you'd have to use new or a static member of Rectangle like:

...
public static Rectangle Widen(Rectangle rect, int widenByAmount)
{
    return new Rectangle(rect.Height, rect.Width + widenByAmount);
}
...

If you have LinqPad try the following:

void Main()
{
    Rectangle r = new Rectangle(5, 10);
    r.Height.Dump();
    r.Width.Dump();
    r.Area.Dump();
    r.Area.Dump();
    r = Rectangle.Widen(r, 10);
    r.Area.Dump();
    r.Area.Dump();
}

// Define other methods and classes here
public class Rectangle
{
    private readonly Lazy<int> _area;

    public Rectangle(int height, int width)
    {
        Height = height;
        Width = width;
        _area = new Lazy<int>(() =>
            {
                "Calculating...".Dump();
                return Height * Width;
            });
    }

    public int Height { get; private set; }

    public int Width { get; private set; }

    public int Area
    {
        get
        {
            return _area.Value;
        }
    }

    public static Rectangle Widen(Rectangle rect, int widenByAmount)
    {
        return new Rectangle(rect.Height, rect.Width + widenByAmount);
    }
}

For me it spits out:

5
10
Calculating...
50
50
Calculating...
100
100

Upvotes: 0

Mat&#237;as Fidemraizer
Mat&#237;as Fidemraizer

Reputation: 64933

Did you know property accessors can have access modifiers?

For example:

public string A
{
    internal get
    {
        // Whatever
    }
    private set
    {
        // Whatever
    }
}

This works for auto-properties too:

public string A
{
    internal get;
    private set;
}

UPDATE: Ok, now you know about accessor access modifiers, what about this?

public class rectangle()
{
    public rectangle(int height, int width)
    {
        Height = height;
        Width = width;
    }

    public int Height { get; private set; }
    public int Width { get; private set; }
    public int Area { get; private set; }

    public void resize(int heightOffset, int widthOffset)
    {
        Height += heightOffset;
        Width += widthOffset;

        RefreshArea();
    }

    private void RefreshArea() 
    {
        Area = Height * Width;
    }
}

Goodbye, private fields!

Upvotes: 4

itsme86
itsme86

Reputation: 19496

A way that I normally do it is like this:

class Rectangle
{
    int _height;
    public int height { get { return _height; } set { _height = value; _area = null; }}

    int _width;
    public int width { get { return _width; } set { _width = value; _area = null; } }

    int? _area;
    public int area
    {
        get
        {
            if(!_area.HasValue)
                _area = _width * _height;
            return _area.Value;
        }
    }
}

Upvotes: 0

Brad Rem
Brad Rem

Reputation: 6026

You said that you modify the private fields from many places in your code instead of always going through the property for those fields. Best case scenario, you limit access to only call the properties and then you are assured you only have a couple places calling the recalculation.

But, if you can't or won't do that, then you are right, it would be a pain to make sure that every time you modified one of your private fields you would have to make sure you calculated your corresponding field.

You also said that performance is an issue and you would like to save on calling the calculation routine.

So, if you must retain access to the private fields, perhaps you could make your recalculation conditional on when those private fields actually do change. Like this:

private int _height; 
private int height 
{ 
    set 
    { 
        _height = value; 
    } 
} 

private int _width; 
private int width 
{ 
    set 
    { 
        _width = value; 
    } 
} 

private int _area; 
// have some variables to store the values used in the calculation
private int _areaWidth;
private int _areaHeight;
public int area 
{ 
    get 
    { 
        // only do the calculation if these values have changed
        if(_areaWidth != _width || _areaHeight != _height)
        {
            _areaWidth = _width;
            _areaHeight = _height;
            _area = _width * _height;
        }
        return _area; 
    } 
} 

This way, you keep your calculation in one place, and you only recalculate when the values have changed. Doing a conditional check is less expensive than whatever your calculation really is.

Upvotes: 0

Jon Senchyna
Jon Senchyna

Reputation: 8037

Your suggested method will work, as long as you make sure that you always use the height/width properties to set _height and _width.

There's nothing wrong with using private properties for private variables. It moves update code to a single location so that it's easier to maintain.

As Matías stated, you could make your height and width properties have accessors with different access modifiers, so that you could read, but not write to your height/width properties outside of this class.

Upvotes: 0

Related Questions