Raheel Khan
Raheel Khan

Reputation: 14787

Using public readonly fields for immutable structs instead of private field/public getter pairs

This is my first time writing small immutable structs that will be used in extensive geometric calculations. I'm tempted to use public readonly fields instead of private field/public getter combinations.

public struct Vector4
{
    public readonly float Angle, Velocity;

    // As opposed to:
    private float _Angle, _Velocity;
    public float Angle { get { return (this._Angle); } }
    public float Velocity { get { return (this._Velocity); } }

    public Vector4 (float angle, float velocity)
    {
        // Once set in the constructor, instance values will never change.
        this.Angle = angle;
        this.Velocity = velocity;
    }
}

It looks a lot cleaner and eliminates an extra layer (the getter). Short of using public fields being bad practice, are there any negative implications to using public readonly fields this way?

Note, we're talking about value types only. Arrays, for example would expose elements to be overwritten by calling code.

UPDATE:

Thanks for all the input. It seems that there is no downside to using public readonly fields for cases like this where data-binding, etc. is not used. In my benchmarks, execution time dropped by 70% which is a big deal. Targeting .NET 4, I would have expected the compiler to inline getter-only properties. The benchmarks were of course tested in release configuration without the debugger attached.

Upvotes: 4

Views: 1604

Answers (4)

Edward Brey
Edward Brey

Reputation: 41728

The .NET Framework Design Guidelines say about fields:

DO NOT provide instance fields that are public or protected.

That's a strong statement. It's not an absolute rule, but it's pretty close per the guidelines' introduction:

There might be situations where good library design requires that you violate these design guidelines. Such cases should be rare, and it is important that you have a clear and compelling reason for your decision.

So what does this mean for your code? A key question is your code's audience. Are you writing a "framework" as the guidelines define it, which is one of the "libraries that extend and interact with the .NET Framework"?

If you're writing a framework, you have constraints that most code doesn't have. Lots of other code will depend on yours, be sensitive to breaking changes, and require binary compatibility to your DLL. This is why properties are important for frameworks. If a later version of a framework needs to add logic to a getter or setter, changing from a field to a property would break binary compatibility.

Most code isn't part of a framework. Dependencies are few and can be easily fixed if there's a simple breaking change. For example, dependent code might use a field in an out or ref parameter. If later that field is changed to a property so that it can have some logic, it's often no big deal to fix the broken code. The compiler will lead you right to the problem. Similarly, there's no need for binary compatibility. You'll won't ever drop in an updated DLL without also recompiling the dependency.

For non-framework code, a field is preferable if the chance of changing it to a property later is small. Fields are more flexible and properties and in some cases faster.

Upvotes: 1

Tim Long
Tim Long

Reputation: 13798

Auto-properties may help.

public struct Vector4
    {
    public float Angle { get; private set; }
    public float Velocity { get; private set; }

    public Vector4(float angle, float velocity) : this()
        {
        // Once set in the constructor, instance values will never change.
        this.Angle = angle;
        this.Velocity = velocity;
        }
    }

[Note: updated to make it compile - thanks Lasse V. Karlsen for pointing that out]

Note that this is exactly the same as having a property with a backing field, except that you are letting the compiler choose its name (which will be some 'unutterable' string of characters).

What is it that bothers you about properties? If it is the verbosity, then use an auto property as above. If you are concerned about performance, then have you measured the impact of properties vs. fields?

Interestingly, in his book CLR via C#, Jeffrey Richter is fairly "down" on properties, stating that he wishes they had not been included in the CLR. He says he prefers explicit use of getXXX and setXXX methods. He argues that the property syntax is confusing. Personally, having seen Java code which works exactly like that, I prefer the property syntax.

I think it is also worth mentioning that readonly can be confusing as well. In this situation it is unambiguous because we are dealing with value types throughout and readonly 'does what it says on the tin'. However, had these been reference types, then readonly only protects the reference - that is, the field must always reference the same object, but the referenced object itself might be mutated if it is written to allow that. I suspect many inexperienced programmers get tripped up by that subtlety.

Given your measured performance benefit, I think you have a reasonable argument for using public readonly fields, and that is a judgement call. The trade-off is that you are allowing consumers of your type to be tightly coupled with the internals of your struct, essentially sacrificing encapsulation. For such a simple type under controlled conditions and with a proven performance benefit, that may well be justified.

Upvotes: 0

dcastro
dcastro

Reputation: 68750

Guidelines, such as "Prefer public properties over public fields" are guidelines, not rules.

In some cases the pros of using fields may outweigh the cons. In fact, Rico Mariani did a good job at illustrating one such scenario, which in my opinion looks a lot like yours:

His main argument for having fields instead of properties was that primitives like Point, Vector and Vertex usually have no illegal values, so there's little or no need for adding the getter/setter layer.

He also makes good arguments for having mutable fields, but that's not your case anyway.

But I'd like to add a point myself for you to consider: Will your classes ever be used for data binding? Data binding only works for properties, not fields.

Upvotes: 6

user743382
user743382

Reputation:

In pure C# without reflection, there are few reasons to avoid read-only fields in your case, and I probably would opt for read-only fields myself. Most of the general advantages of properties don't really apply here. That said...

Anything that uses reflection to get a list of properties, and acts on those properties, will not work with fields (whether read-only or not) without modification.

In particular, changing properties to fields may cause data binding to stop working. It will continue to compile without any issue, but it will no longer do what you want it to. If you have any such code, or you anticipate such code in the future, you need to keep using properties.

Upvotes: 6

Related Questions