Why are internal fields preferable to protected fields in an abstract class?

Question

I have an abstract class for a device that includes a serial port:

public abstract class SerialDevice
{
    // serial port (should this be protected, internal, or protected internal?)
    protected SerialPort _serialPort;

    // The serial port has some shared methods.
    public void Open()
    {
        _serialPort.Open();
    }
}

Derived classes also use the serial port, but I don't want the port visible outside of those derived classes.

public class Widget : SerialDevice
{
    // The serial port has some widget-specific functionality.
    public void UniqueCommand()
    {
        _serialPort.WriteLine("Hello world.");
    }
}

When I compile, I get this warning: CA1051: Do not declare visible instance fields, which says that fields should be a hidden implementation detail. Ok, I can use a protected property instead and safeguard my classes against changes that break stuff.

But it goes on to recommend fields be private or internal. The C# Programming Guide's page on Access Modifiers says that making the serial port "protected" allows it to be accessed only within derived classes, while making it "internal" would allow it to be accessed within the whole assembly. So why is "internal" acceptable, but "protected" isn't? Isn't protected access less visible than internal access?

Related question: What should the accessablity of Fields in a Abstract Class be?

Your class is public, has a public constructor (since your question does not show a constructor, we should assume that it has a default constructor) so any class in any assembly can extend your class and can access the protected member. That's why it complains. — Oguz Ozgul, May 01 '20 at 22:26
@OguzOzgul that makes sense. I cannot make the abstract class private, else I get error CS1527 (Elements defined in a namespace cannot be explicitly declared as private, protected, protected internal, or private protected). Should it be *internal*, or something else? — adamj537, May 02 '20 at 10:56
If I make it *internal* I get CS0060 (Inconsistent accessibility: base class 'SerialDevice' is less accessible than class 'Widget'). What's the solution? — adamj537, May 02 '20 at 11:02
Let the SerialDevice be public, this is how we expose internal classes extending our internal abstract or base classes to the outside world. Just make it's constructors internal. Keep the _serialPort internal — Oguz Ozgul, May 02 '20 at 13:25

score 1 · Answer 1 · answered May 01 '20 at 22:30

1

In a way protected is more visible then internal since you can access the field outside of the assembly if you extend the class but internal restricts it to the one assembly where the field is defined.

answered May 01 '20 at 22:30

VeithBürgerhoff

68
5

score 1 · Accepted Answer · answered May 02 '20 at 13:35

To achieve encapsulation, we should not expose our fields directly to the outside, even to a class inside our own assembly.

Instead, we can have an internal getter method so that only classes inside our assembly can access it, use it, but cannot change its reference.

If we go one step further and want the abstract/base class to be extendible only by classes in our own assembly:

By having the base class public, and specifying all constructors as internal, we can make sure that only classes inside our own assembly can extend it.

public abstract class SerialDevice
{
    private SerialPort _serialPort;

    internal SerialPort GetSerialPort()
    {
        return _serialPort;
    }

    // We define our base class public,
    // but also define all the constructors internal
    // Therefore, no class outside our assembly extend it
    // but we still can expose sub classes (like Widget) to the outside
    internal SerialDevice()
    {

    }

    // The serial port has some shared methods.
    // These methods are still visible from the outside
    public void Open()
    {
        _serialPort.Open();
    }
}

And,

public class Widget : SerialDevice
{
    // The serial port has some widget-specific functionality.
    public void UniqueCommand()
    {
        GetSerialPort().WriteLine("Hello world.");
    }
}

adamj537 · Answer 3 · 2020-05-05T17:42:27.557

The answer by Oguz works well, but I decided to use a property instead of a method. So my solution looks like this, and appears to comply with best practices.

The abstract class is public so the rest of the assembly can see it.
The abstract class' constructor is internal so it can't be inherited outside of the assembly.
The serial port, "Port," is a property, not a field, so that it can be changed later within the abstract class without breaking any of the derived classes.
The property "Port" is protected so it cannot be seen except within classes derived from the base class.

public abstract class SerialDevice
{
    // Constructor
    internal SerialDevice() {}

    // serial port
    protected SerialPort Port { get; } = new SerialPort();

    // The serial port has some shared methods.
    public void Open()
    {
        Port.Open();
    }
}

public class Widget : SerialDevice
{
    // The serial port has some widget-specific functionality.
    public void UniqueCommand()
    {
        Port.WriteLine("Hello world.");
    }
}

Why are internal fields preferable to protected fields in an abstract class?

3 Answers3