Are protected properties evil?

Question

Introductory piece of code:

class BaseClass
{
    protected Foo MyFoo { get; }
}

class ChildClass : BaseClass
{
    void SomeMethod()
    {
        MyFoo.DoStuff();
        //Here, I have no idea that MyFoo is not defined in this class,
        //but rather in the base class.
    }
}

The problem/debate is that you have to either know or check manually where MyFoo is defined. And you're better off using two private fields, one in the base class, on in the child class.

In our case, most of the time, these properties come from dependency injection and are available in the constructor of both classes. So it's really easy to just map them to private properties from that constructor, instead of inheriting it from the base class, which is also less clear.

Is there good practice that I'm missing? What is your experience there?

Underlying question: Are protected properties just a bad habit, and should I always go for private fields instead?

Will this go against me when unit testing?

Is this considered clean/unclean code?

Answer 1

I don't think duplicating the property in the subclass (and its constructor) is really a better alternative. For actual read/write properties, that can't even yield the same functionality, and for read-only injected properties as in your case (some kind of service probably) it does not matter that much where it's defined. Besides, your IDE can easily show you that.

Basically, the way to look at it is this:

• Protected fields are part of the class API for subclassing, just like methods.
• Protected fields that actually contain mutable data can definitely result in code that is hard to understand, but so can contain protected methods when they are overridden in subclasses but called in the superclass.
• The problem here is not protected fields or methods, the problem is subclassing. It should be used carefully and sparingly.
• A rule of thumb I just made up on the spot: either the superclass should be very small and simple, or the subclass. And deep inheritance hierarchies are bad.

Answer 2

you have to either know or check manually where MyFoo is defined

But how is that specific to protected properties? The same is true for methods and really any public member. One big reason for inheritance is to avoid having to repeat the same thing for every concrete type. So you can have a common base type that contains base behavior and base storage definitions.

A property or field being protected just tells you that this is an internal implementation detail which is likely to be relevant for subclasses. In a way, this actually tells you more than having completely unconnected private properties on each level.

And the compiler already knows where it is defined and how its accessible. And good editors will tell you that at design time, so there is really no need to “check manually”. In addition, when inheriting a type, I would expect you to be comfortable with its interface, so you know that there are protected members you can use.

In our case these properties come from dependency injection

This might be just my opinion, but I would always use readonly fields for dependencies, not properties.

So it's really easy to just map them to private properties from that constructor

Sure, you could have a private field on each level of your type hierarchy. But that will also duplicate the memory usage you need to keep track of those. And if you for a moment ignore the fact that these are unchanging dependencies injected into the constructor, a base class not being able to change the value for its subclasses might also introduce problems.

Are protected properties just a bad habit, and should I always go for private fields instead ?

There are two questions here: Property vs. field, and protected vs private. To answer the latter: Use what makes sense in an encapsulation sense. If your base type has actual internal APIs that subclasses could use, those should be protected. But you shouldn’t just make any dependency protected just in case.

Answer 3

Are protected properties evil?

No. Protected properties are smelly, but not evil. That is, they should make you think "am I modeling this correctly?" One thinks of a property as being part of the public surface area of a model, and a field as being a private implementation detail of a model. Since "protected" simply means "a private implementation detail of this hierarchy", one expects protected fields rather than protected properties.

That said, protected properties are not wrong; they're just a sign that perhaps you need to think about your design a little harder.

Here, I have no idea that MyFoo is not defined in this class, but rather in the base class.

Of course you know that. You can read the source code of the class and plainly see that there is no such property. And you can "go to definition" and go to the source code for the property.

The problem/debate is that you have to either know or check manually where MyFoo is defined.

No, you don't. Why do you think you have to know where it is defined?

And you're better off using 2 private fields, one in the base class, on in the child class.

No, I don't agree with that at all. You'd possibly be better off using a protected field in the base class.

Are protected properties just a bad habit, and should I always go for private fields instead ?

If you find yourself writing a lot of protected properties, I would ask yourself if they are actually better modeled as protected fields. Remember, a property should model a property of the modeled thing. If you're writing a Car class then its properties should be the properties of a car. What things are both properties of a car and also the implementation details of the hierarchy of cars? I can't think of anything that is both those things, and so maybe there ought not to be protected properties.