Best way to expose protected fields

Question

I have a base class like this:

 public class BaseModalCommand
 {

    protected object m_commandArgument;
    protected int m_commandID;
    protected int m_enableUIFlags;

    public virtual void OnIdle()
    {
    }

    public virtual void OnResume()
    {
    }

    public virtual void OnStart(int commandID, object argument)
    {
    }

    public virtual void OnStop()
    {
    }



    public virtual int EnableUIFlags
    {
        get
        {
            return this.m_enableUIFlags;
        }
    }
}

The virtual methods are to be overridden in derived types. If I run it thru FxCop, it complains about not declaring visible instance fields and recommends changing it to private and exposing it as a protected property.

Any thoughts? I'm thinking this message can be ignored.

Answer 1

The advice of FxCop is sound. You don't want to expose protected fields to derived classes direclty. The state managed by the base class should be managed by the base class - never directly modified by any derived class.

Answer 2

Basically, FxCop recommends that you should do

private object m_commandArgument;

protected object CommandArgument
{
   get { return m_commandArgument; }
   set { m_commandArgument =value}
}

This is based on OO encapsulation rule- (One of three OO rules). You may want to check the value before assigning, and you want to ensure this is not directly manipulated by the derived class.

Answer 3

Use properties. Change the member variables to private and then setup protected properties for each member variable.

Regards,
Frank

Answer 4

As best practice, your class fields should be marked as private and wrapped in a getter/setter property

so instead of

protected object m_commandArgument;

use

private object m_commandArgument;

protected object CommandArgument {get; set;}

Theres several advantages to this but a simple usage would be exception handling/validation in your setter.

e.g.

private string _email;
protected string Email
{ 
   get { return _email; }
   set 
   {
       if(value.IndexOf("@") > 0)
           _email = value;
       else
            throw new ArgumentException("Not a valid Email");
   }
}

Answer 5

For any class, there are two kinds of uses by client code: code that references your class, and code that inherits your class. It's widely recognized that the second kind of use by far the most tightly coupled. Changes in your class directly affect their internal mechanics. Your exposing protected members like this means that changes in your base class will affect how your derived classes work in ways that are unpredictable without comparing the code of each base and derived class. Equally bad, your derived classes can modify the internals of the base class.

If you really want to expose internal data members like this, wrap private data members in protected properties (as gisresearch suggests). These properties (along with any protected methods) constitute the inheritance interface of your class. Like any interface exposed to outside clients (whether through just defining public methods and properties, or through explicit interface implementation), this interface is something you will need to manage, particularly in larger code bases. An interface can be changed, but should be changed as a conscious decision.

If you work with protected data members directly, you have much less control over the dependencies between base and derived classes. Trust me on this: not even being able to determine what effects a change may have can be a Very Unpleasant Thing.

Answer 6

Paul Alexander is correct and so is FxCop.

You want to make your fields private and expose them through a property to prevent derived classes from changing the variable by themselves. Forcing them to go through a property gives the base class a chance to validate and/or reject any modifications.