C# protected field to private, add property--why?

Question

In Visual Studio 2008 Team System, I just ran Code Analysis (from the Analyze menu) on one of my C# projects. One of the warnings produced was the following:

Microsoft.Design : Because field 'Connection._domain' is visible outside of its declaring type, change its accessibility to private and add a property, with the same accessibility as the field has currently, to provide access to it.

It's referring to the following field:

public abstract class Connection
{
    protected string _domain;
}

I don't understand the reasoning behind the suggestion. This is what I think it wants me to do:

public abstract class Connection
{
    private string _domain;
    protected string Domain { get { return _domain; } set { _domain = value; } }
}

Two questions:

1. Did I understand correctly what the suggestion wants me to do, code-wise?
2. Why does it want me to do this?

Answer 1

Yes, I think you understood correctly - although in later versions of C#, there's a more concise way to write it:

public string Domain { get; set; }

Why? It's all about encapsulation. If you do as it suggests, you can later change the definition of the Domain property without affecting any calling code that uses that property. Since your class is public, and might conceivably be called by code that you didn't write, that's potentially quite important.

Answer 2

Yep. That's the suggestion. You shouldn't have any accessibility higher than private exposed as direct instance fields.

It's one of the main principles of OOD - encapsulation also referred to as 'data-hiding'.

Answer 3

1. Yes, you did correct the problem code wise.
2. It is about encapsulation. _domain is data about your object. Rather then exposing it directly so that any client has unfiltered access, you should provide an interface for them to access it. Practically this might be adding validation to the setter so that it can't be set to any value. It might seem silly if you are the only one writing code because you know how your API works. But try to think about things on a large enterprise level, it is better to have an API so that your object can be seen as a box that accomiplishes a task. You might say you will never have the need to add something like validation to that object, but things are done that way to hold for the possibility of it, and also to be consistent.

Answer 4

This is because if you ever wanted to change the field to a property in the future you would break any other assemblies that depend on it.

It is good practice to keep all fields private and wrap them in properties so that you have the option of adding validation or other logic in the future without recompiling all consumers (or in this case inheritors) of your class.

Answer 5

Your translation is correct. The same argument for can be made for using 'protected' properties as can be made for using 'public' properties instead of exposing member variables directly.

If this just leads to a proliferation of simple getters and setters then I think the damage to code readablity outweighs the benefit of being able to change the code in the future. With the development of compiler-generated properties in C# this isn't quite so bad, just use:

protected string Domain { get; set; }

Answer 6

In answer to your question... yes.

However, I would just use the auto-property syntax:

public abstract class Connection
{
    protected string Domain { get; set; }
}

Answer 7

Basically, properties provide more than returning or setting a member. They allow you to add logic that could verify a proper input format, range validation, etc.

The selected answer from the link puts it best, "Properties provide encapsulation. You can encapulate any needed validation/formating/conversion in the code for the property. This would be difficult to do for fields."

http://social.msdn.microsoft.com/Forums/en-IE/netfxbcl/thread/985f4887-92ae-4ec2-b7ae-ec8cc6eb3a42

Answer 8

In addition to the other answers mentioned here, public/protected members that begin with an underscore are not CLS-compliant, in that there is no requirement for .NET languages to support members with leading underscores, so someone inheriting from your class in a different .NET language may not be able to access that particular protected member.

I know, it probably doesn't apply to you, but it might be part of the reason for the code analysis warning.