Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.

Reading Effective C++, Item 31 describes two methods for minimizing recompilation time by having clients use one class which only provides an interface to functions they can call. This class refers those function calls to other classes that do the real work.

The first method described is the Handle class, which uses the pImpl idiom to forward function calls to an implementation class, which contains the implementation of everything it's doing. The Handle class has a pointer to an instantiated object of this type, and its functions call the corresponding functions on that object and return their values.

The one I'm having problems with is the second one, where this is implemented using a base class with all pure virtual functions. In this method, the base class defines what functions are allowed to be called, but they are all declared as pure virtual, and so the derived class is what actually implements them. Clients have pointers or references to the base class which point to derived class objects, and call those functions, which map to the derived class' implemented functions. The book describes having the Base class have a factory function which returns a pointer to the derived class. From the book's example, if Person is the base class and RealPerson is the derived class, clients should be able to create an object like this:

std::string name;
Date dateOfBirth;
Address address;

//create an object supporting the Person interface
std::tr1::shared_ptr<Person> pp(Person::create(name, dateOfBirth, address);

When I tried to implement something similar, built around a Rational class, I ran into problems, however:

class Rational;

class RationalBase {
public:
    virtual ~RationalBase ();

    static RationalBase* create(int top, int bottom) {
        RationalBase* ptr = new Rational(top, bottom);
        return ptr;
    }

    virtual int getNumerator () const = 0;
    virtual int getDenominator () const = 0;
    virtual void setNumerator (int) = 0;
    virtual void setDenominator (int) = 0;
};

class Rational : public RationalBase {
public:
    Rational (int top, int bottom) : numerator(top), denominator(bottom) {}
    virtual ~Rational () {}

    virtual int getNumerator () const {
        return numerator;
    }

    virtual int getDenominator () const {
        return denominator;
    }

    virtual void setNumerator (int top) {
        numerator = top;
    }

    virtual void setDenominator (int bottom) {
        denominator = bottom;
    }

private:
    int numerator;
    int denominator;
};

The g++ compiler is giving me the error invalid use of incomplete type ‘struct Rational’ on the line RationalBase* ptr = new Rational(top, bottom); inside the create function.

Googling this error, I found this question which explains that the reason for this is that you're only allowed to do so much with forward-declared classes, and creating new instances of them is not one of them. But then I don't understand how the Interface class is supposed to be implemented?

share|improve this question

1 Answer 1

up vote 3 down vote accepted

Right, you cannot do RationalBase* ptr = new Rational(top, bottom); before Rational is defined because the compiler doesn't know what that type is yet. (It doesn't know how much space to allocate, or what constructor to call yet). Personally, I would not use a factory method for this.

I would just have the user write:

RationalBase* ptr = new Rational(top, bottom);

themselves as this makes things fairly obvious.

If you want to use smart pointers, then they can of course write that too.

std::shared_ptr<RationalBase> ptr(new Rational(top, bottom));
std::unique_ptr<RationalBase> ptr(new Rational(top, bottom)); // ensure only one pointer to this object

or better yet in c++11 :-)

auto ptr = std::make_shared<Rational>(top, bottom); // C++11
auto ptr = std::make_unique<Rational>(top, bottom); // C++14 only

If you really want to use a factory, then you can change the order of things slightly and move the create function to the bottom:

class Rational;

class RationalBase {
public:
    virtual ~RationalBase () {} // you needed to provide a body to this function

    static RationalBase* create(int top, int bottom);

    virtual int getNumerator () const = 0;
    virtual int getDenominator () const = 0;
    virtual void setNumerator (int) = 0;
    virtual void setDenominator (int) = 0;
};

class Rational : public RationalBase {
public:
    Rational (int top, int bottom) : numerator(top), denominator(bottom) {}
    virtual ~Rational () {}

    virtual int getNumerator () const {
        return numerator;
    }

    virtual int getDenominator () const {
        return denominator;
    }

    virtual void setNumerator (int top) {
        numerator = top;
    }

    virtual void setDenominator (int bottom) {
        denominator = bottom;
    }

private:
    int numerator;
    int denominator;
};

// note that this is defined AFTER the Rational class
RationalBase* RationalBase::create(int top, int bottom) {
    RationalBase* ptr = new Rational(top, bottom);
    return ptr;
}

I'm assuming that you are aware that you can provide the bodies of functions separately from the declaration of the them. Doing so allows you to address these types of "order of definition" problems.

share|improve this answer
    
std::unique_ptr seems more appropriate tbh. –  miguel.martin Jul 30 '14 at 16:10
    
@miguel.martin, sure, if the user knows they want only one pointer to it. –  Evan Teran Jul 30 '14 at 16:11
    
Neither shared_ptr nor unique_ptr are appropriate here, @miguel.martin. Why are pointers used here at all? That makes no sense whatsoever. Rational is obviously a value. –  Konrad Rudolph Jul 30 '14 at 16:17
    
@Evan Teran: Thanks. All good ways of doing it. I'm still a junior-level programmer, so I didn't understand why it wasn't working when I was doing it basically the way the book said to, and I was just trying to understand if there's some standard way this is supposed to be implemented. –  A. Duff Jul 30 '14 at 16:19
    
@KonradRudolph: you may be right about it being a value. However, I am making no judgments on the appropriateness of using a pure-virtual base class and pointers here. I think that is an exercise left to the developer himself. For now, I was just explaining how to implement the pattern itself. –  Evan Teran Jul 30 '14 at 16:55

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.