Is a public constructor in an abstract class a codesmell?

Question

Is a public constructor in an abstract class a codesmell? Making the constructor protected provides all of the access of which you could make any use. The only additional access that making it public would provide would be to allow instances of the class to be declared as variables in scopes that cannot access its protected members, but instances of abstract classes cannot be declared at all.

Answer 1

As you say, there is no need to provide public constructor of abstract class, nor it can be misused, if you provide public constructor.

However, you may consider declaring constructor as public as recomendation for structuring classes derived from the abstract one.

Answer 2

I've read it in at least one coding guideline that constructors of abstract classes should not be public - I think that rule makes sense for the reason you gave.

However, i can't imagine a scenario where making it public would make things go wrong. So i wouldn't go so far as saying it's a code smell. I see the protected constructor as a "nice to have" property :)

Answer 3

My opinion would be that the public constructor might be seen to be confusing, and as you say making it protected would be correct. I would say that a protected constructor correctly reinforces the impression that the only sensible use of an abstract class is to derive from it.

In fact, you only need to declare a constructor in an abstract class if it needs to do something, eg. initialise its own private members. I would then expect there to be other protected member functions that are helpful to derived classes.

EDIT:

Since no-one has posted any code and @sbi asked for some in a comment to OP, I thought I would post some:

class Base:
{
public:           // The question is: should the ctor be public or protected?
// protected:
    Base():i(0){} // ctor is necessary to initialise private member variable
public:
    virtual ~Base(){} // dtor is virtual (but thats another story)
                  // pure virtual method renders the whole class abstract
    virtual void setValue(void)=0;  
protected:
    int getValue(void){ return i;}
private:
    int i;
};

Base b1;  // Illegal since Base is abstract, even if ctor is public
Base& b2=makeBase();  //We can point to, or refer to a Base
b2.setValue();    // We're not sure what this does, but we can call it.
b2.getValue();    // Illegal since getValue is protected

Answer 4

Be careful though to explicitly declare/define your protected/private copy constructor. If you don't the compiler will do it for you. See scott meyers effective c++ item 6.

Answer 5

I'd say its a bad sign for a couple of reasons.

The first is that it might mislead somebody into thinking they can actually call that routine.

The second is that it implies the author thought you could call it. You now don't know if that was an important part of his design or not.

Answer 6

I know that allot of people obviously disagree, but I don't think an abstract classes ctor should be protected unless the interface you are designing has a specific reason for all derived classes to also have a protected ctor.

The reason is that when you design an abstract class you're designing an interface to be implemented by the deriving classes. Derived classes should implement the interface exactly as it's designed in the base class. The interface presented by an abstract base class is a contract; don't break that contract.

Or in other words, if you're changing the interface, why are you deriving from that class in the first place?

This is a bit of a religious answer, but what the heck. ;-)

Answer 7

Abstract class can never be directly instantiated because of the pure virtual methods it declares. So, declaring constructor as protected is just adds additional tips for programmers.

I saw recommendation to declare constructor as protected here, in Google style guide. And I've met similar recommendations in some other corporate coding standards. So this is looks like a good practice.