Ken Keenan
Ken Keenan

Reputation: 10538

Are duplicated private member variables preferable to a shared protected member?

This is an idiom I have noticed in a C# codebase which I have been working on recently:

class Base
{
   private readonly MemberClass _memberVariable;

   public Base(MemberClass memberValue)
   {
       _memberVariable = memberValue;
   }

   // methods accessing memberVariable...
}

class Derived : Base
{
   private readonly MemberClass _memberVariable;

   public Derived(MemberClass memberValue) : base(memberValue)
   {
       _memberVariable = memberValue;
   }

   // methods accessing memberVariable...
}

Both the base class and its derived class have a member variable which is intialised in their respective constructors, but instead of declaring the member as protected in the base class and having it available in its subclasses, each subclass has its own private copy of the member.

My first reaction to this was that it was an unnecessary duplication. On thinking about it some more, I feel that the duplication might be justified as it reduces the amount of coupling between the base and derived classes and prevents a change in the base class causing inadvertent breakage in its subclasses.

Are protected members "considered harmful" in C#?

Upvotes: 1

Views: 126

Answers (2)

Peter Duniho
Peter Duniho

Reputation: 70681

Are protected members "considered harmful" in C#?

Members in general? No, for sure. There are lots of good reasons to have protected members in C# code.

Furthermore, I think that clearly the code you've used in your example is broken. Having two copies of the exact same value all the time is wasteful and if anything, is likely to create bugs (as soon as you have two fields that are supposed to have the same value, you now have the opportunity for those values to be not the same and thus wind up breaking some code somewhere).


As far as the question of protected fields specifically? I will dissent from Jon's opinion here, at least to a degree: for a readonly field, IMHO it is perfectly fine in some situations.

The main reason for encapsulating such a field in a property would be so that the implementation could be elaborated on later without having to recompile dependent code (assuming that code is in a different assembly).

But IMHO such a field is very likely to never need elaboration on its implementation. Such fields are usually intended as sources for constant value. A getter with something more elaborate than just returning a field value is, while not exactly rare, not all that usual either (the biggest exception I can think of being lazily-initialized values).

For now, most of us are using C# 5 or earlier, and that's likely to be true for some time now. Until the C# 6 syntax is available widely, I'd say that using a readonly protected field is a fine concession to conciseness without a major maintenance/correctness hazard in certain specific scenarios.

Naturally, the more complicated the code gets, the more useful good, safe abstractions will be, and the more likely you should stick with a truly encapsulated read-only property.

But as an absolute rule? No, I don't think it's necessary to have a blanket prohibition against readonly protected fields.

Upvotes: 1

Jon Skeet
Jon Skeet

Reputation: 1501143

I would consider protected fields harmful - and I would also consider the data duplication harmful, if the values should always be the same. However, the base class could expose the private field's value via a property instead:

class Base
{
   private readonly MemberClass memberVariable;

   protected MemberClass MemberProperty { get { return memberVariable; } }    

   public Base(MemberClass memberValue)
   {
       this.memberVariable = memberValue;
   }

   // methods accessing memberVariable or MemberProperty...
}

class Derived : Base
{
   public Derived(MemberClass memberValue) : base(memberValue)
   {
   }

   // methods accessing MemberProperty...
}

In C# 6, the base class becomes simpler:

class Base
{
   protected MemberClass MemberProperty { get; }    

   public Base(MemberClass memberValue)
   {
       this.MemberProperty = memberValue;
   }

   // methods accessing MemberProperty...
}

That's still a protected property backed by a private readonly field - it's just the compiler does all the boilerplate for you.

Upvotes: 3

Related Questions