tags:

views:

321

answers:

7

I have an interface class similar to:

class IInterface
{
public:

    virtual ~IInterface() {}

    virtual methodA() = 0;

    virtual methodB() = 0;

};

I then implement the interface:

class AImplementation : public IInterface
{
    // etc... implementation here
}

When I use the interface in an application is it better to create an instance of the concrete class AImplementation. Eg.

int main()
{
    AImplementation* ai = new AIImplementation();
}

Or is it better to put a factory "create" member function in the Interface like the following:

class IInterface
{
public:

    virtual ~IInterface() {}

    static std::tr1::shared_ptr<IInterface> create(); // implementation in .cpp
    virtual methodA() = 0;

    virtual methodB() = 0;

};

Then I would be able to use the interface in main like so:

int main()
{
    std::tr1::shared_ptr<IInterface> test(IInterface::create());
}

The 1st option seems to be common practice (not to say its right). However, the 2nd option was sourced from "Effective C++".

+4  A: 

One of the most common reasons for using an interface is so that you can "program against an abstraction" rather then a concrete implementation.

The biggest benefit of this is that it allows changing of parts of your code while minimising the change on the remaining code.

Therefore although we don't know the full background of what you're building, I would go for the Interface / factory approach.

Having said this, in smaller applications or prototypes I often start with concrete classes until I get a feel for where/if an interface would be desirable. Interfaces can introduce a level of indirection that may just not be necessary for the scale of app you're building.

As a result in smaller apps, I find I don't actually need my own custom interfaces. Like so many things, you need to weigh up the costs and benefits specific to your situation.

Ash
Thanks Ash. I ended up going with this solution because it provides a clear line between declaration and definitioin. And therefor I only need to include the interface header file when using the interface and not the header file containing the implementation as well.
Seth
+1  A: 

"Better" in what sense?

The factory method doesn't buy you much if you only plan to have, say, one concrete class. (But then again, if you only plan to have one concrete class, do you really need the interface class at all? Maybe yes, if you're using COM.) In any case, if you can forsee a small, fixed limit on the number of concrete classes, then the simpler implementation may be the "better" one, on the whole.

But if there may be many concrete classes, and if you don't want to have the base class be tightly coupled to them, then the factory pattern may be useful.

And yes, this can help reduce coupling -- if the base class provides some means for the derived classes to register themselves with the base class. This would allow the factory to know which derived classes exist, and how to create them, without needing compile-time information about them.

Dan Breslau
I am using COM actually. I have a VCL GUI and I need to drag and drop from Windows Explorer. So I am inheriting IDropTarget in my interface, and I am getting components which wish to be drop targets in the application to register their handle through this interface. So it probably wouldn't be worth using the factory method, and better to use Micahel Safyan's suggestion above.
Seth
A: 

I would go with the first option just because it's more common and more understandable. It's really up to you, but if your working on a commercial app then I would ask what my peers what they use.

Lucas McCoy
+4  A: 

There is yet another alternative which you haven't mentioned:

int main(int argc, char* argv[])
{
   //...
   boost::shared_ptr<IInterface> test(new AImplementation);
   //...
   return 0;
}

In other words, one can use a smart pointer without using a static "create" function. I prefer this method, because a "create" function adds nothing but code bloat, while the benefits of smart pointers are obvious.

Michael Aaron Safyan
If the implementation is supposed to be hidden, will be dynamically loaded, is dependent on the platform (e.g. AWindowsImplementation, AMacImplementation, ALinuxImplementation), or depends on a number of configuration parameters, then you should create a factory. However, IMHO, you should use a separate Factory class for that.
Michael Aaron Safyan
+2  A: 

There are two separate issues in your question: 1. How to manage the storage of the created object. 2. How to create the object.

Part 1 is simple - you should use a smart pointer like std::tr1::shared_ptr to prevent memory leaks that otherwise require fancy try/catch logic.

Part 2 is more complicated.

You can't just write create() in main() like you want to - you'd have to write IInterface::create(), because otherwise the compiler will be looking for a global function called create, which isn't what you want. It might seem like having the 'std::tr1::shared_ptr test' initialized with the value returned by create() might seem like it'd do what you want, but that's not how C++ compilers work.

As to whether using a factory method on the interface is a better way to do this than just using new AImplementation(), it's possible it'd be helpful in your situation, but beware of speculative complexity - if you're writing the interface so that it always creates an AImplementation and never a BImplementation or a CImplementation, it's hard to see what the extra complexity buys you.

aem
+1  A: 

Use the 1st method. Your factory method in the 2nd option would have to be implemented per-concrete class and this is not possible to do in the interface. I.e., IInterface::create() has no idea exactly which concrete class you actually wish to instantiate.

A static method cannot be virtual, and implementing a non-static create() method in your concrete classes has not really won you anything in this case.

Factory methods are certainly useful, but this is not the correct use.

Which item in Effective C++ recommends the 2nd option? I don't see it in mine (though I don't also have the second book). That may clear up a mis-understanding.

Mortimer
Item 31, I have the Third edition
Seth
I don't have my Effective C++ handy, but I think the OP is referring to the Abstract Factory pattern (which I always thought of as overkill, but maybe there are times when it's appropriate.)As for whether the factory needs to be implemented for each concrete class: You'd have a single public factory method, which invokes non-public static methods to create the concrete instances. The derived classes register themselves with the factory to make themselves available for construction. How the factory chooses **which** derived class to build at a given time is implementation-specific.
Dan Breslau
but you are right about the create() method. in the book it is actually test(IInterface::create())
Seth
A: 

I do have a very simple question there:

Are you sure you want to use a pointer ?

This question might seem unlogical but people coming from a Java background use new much often than required. In your example, creating the variable on the stack would be amply sufficient.