Is an abstract class with fields considered not a good practice?

Question

I created an abstract class that implements an interface. This abstract class will be the base of several concrete classes that need to populate the properties of that interface.

The CLR compliance warnings pop up on the first two examples. I understand what they represent and there are several questions here that cover them.

To make the field different I can add a trailing underscore. It is accepted by the compiler. Is this a correct style choice. I don't think it stands out very well and may be a code smell. But I may just not be used to it.

Or am I wrong to make an abstract ancestor that defines property fields? The idea of course is to save duplication of work and help to enforce a standard implementation, but I can see that it might have its own smell in the descendent when it starts assigning values to these "hidden" fields.

namespace MyLittleCompany.Widgety
{
  public abstract class MlcWidgetInformation : IMlcWidgetInformation
  {
    //Compiler complains of Non-CLR Compliance (case difference only)
    protected int sides;  //Number of sides for this widget

    //Compiler complains of Non-CLR Compliance (non-private name with underscore
    // is not compliant)
    protected int _hooks; //Number of hooks on this widget

    //Compiler is happy with a trailing underscore
    protected int feathers_; //Number of feathers on this widget

    // Interface items
    public Sides { get { return sides; } } 
    public Hooks { get { return _hooks; } }
    public Feathers { get { return feathers_; } }
  }
}
=====================================
namespace MyLittleCompany.Widgety
{
  public class SmallWidgetInformation : MlcWidgetInformation 
  {
    public SmallWidgetInformation()
    {
      // Is this a smell? As in "What are these things?"
      sides = 6;
      _hooks = 3;
      feathers_ = 1;
     }
  }
}

it's not the compiler that is complaining, it is some style checker tool (eg. resharper) — thumbmunkeys, Jul 09 '15 at 16:31
it is also good practice to postfix the abstract class name with `Base`, eg `MlcWidgetInformationBase` — thumbmunkeys, Jul 09 '15 at 16:33
@thumbmunkeys Fair enough. It would be the native analyzer that is in VS 2013 Update 4. — Rich Shealer, Jul 09 '15 at 19:13

score 3 · Answer 1 · answered Jul 09 '15 at 20:22

Having to create an abstract base class just to avoid repeatedly defining three fields isn't a code smell, but:

It does feel like taking DRY to extremes,
and (assuming you use inheritance elsewhere), you block the opportunity to inherit from other classes.

However, if you are willing/able to use VS2015 and C# 6, help is at hand. The new read-only auto properties allow you to do this, removing the need for the base class without repetition:

public interface IMlcWidgetInformation
{
    int Sides { get; }
    int Hooks { get; }
    int Feathers { get; }
}

public class SmallWidgetInformation : IMlcWidgetInformation
{
    public int Sides { get; } = 6;
    public int Hooks { get; } = 3;
    public int Feathers { get; } = 1;
}

Until C# 6 becomes more widely adopted, you are left choosing between inheritance and repeating yourself.

In reality the class I'm working with has 11 properties. The assignments are more complex, but I was keeping it simple for my question. — Rich Shealer, Jul 10 '15 at 00:45

score 2 · Accepted Answer · answered Jul 09 '15 at 16:49

It's absolutely accepted to make protected fields in abstract classes.

Naming conventions are guidelines only and it depends on the styling tool you are using. Use the style that you and/or your team preffer and customize your tool from that. The most important thing is that the project is consistent in its own.

I've personally never seen the trailing underscore in use before, but I can see the benefit in it. Might be a pretty smart way to show protected fields. I would definately approve of that convention if I ran into a team that used it.

Is an abstract class with fields considered not a good practice?

2 Answers2