5

So, my Business Code needs some Objects.
It does not know how much objects it needs and it does not know the exact types (because polymorphism is involved).
For me, that sounds for a good reason to go factory pattern.

My code now looks like:

std::vector<AbstractBaseClass*> objectList;
Factory f;
objectList = f.create("path/to/config.txt");

The prototype of the factory's create-method looks like:

std::vector<AbstractBaseClass*> Factory ::create(std::string configFile)

Good news, its working!
The Factory reads the config, and then decides how many objects to create and which concrete types they have.

Well, I did a lot of searching but I couldn't find an example of a factory returning not a single object but a container. Thus, my question: is this good style?

I think yes because this way the whole parsing/creation process is hidden from the business logic. The logic only knows it has some container with a bunch of objects. But maybe you have other opinions?
Please note: this project is all about learning good OOP habits.

Ok, imagine this approach is OK.
I learned, raw pointers are evil. (OK, they are not evil per definition, but I try to avoid them).
So, I want to move to some smart pointers. Lacking boost and C++11 on this machine I'm starting with auto_ptr.

OK, new approach:

std::vector< auto_ptr<AbstractBaseClass> > objectList;
Factory f;
objectList = f.create("path/to/config.txt");

And Factory:

std::vector< auto_ptr <AbstractBaseClass> > Factory ::create(std::string configFile)

This looks evil.
And it doesn't compile because at the moment, I'm getting some crazy STL compiler errors.
But, imagin' it compiles.
Is this good?

I've never seen such a construct - a factory returning a container of smart pointers. And because I'm not that expert, I want to ask what you think about this.

On a related note, what smart pointer shall I use?
The business logic is the only owner of the objects, so I guess unique_ptr. However, I'm not sure if a unique_ptr container can be returned. shared_ptr is easier to implement, I think.

  • 1
    auto_ptr is not the right pointer in nearly all cases. unique_ptr is its replacement. – ratchet freak Dec 15 '15 at 15:30
  • 1
    In particular, standard containers are not allowed to contain auto_ptr. – Sebastian Redl Dec 15 '15 at 23:14
9

If you use C++ as a mostly OOP language, you'll have to deal with pointers in some form or another. The answer to almost all pointer problems is std::unique_ptr, because it has fairly value-like semantics while still enabling you to use polymorphism. Its only overhead is syntactic clutter. This is a lot better than

  • raw pointers, because pointers have no inherent ownership semantics. I've seen pointers being used for both reference-like borrowing semantics, and copy-like ownership transferral. That might be in the documentation, but it certainly isn't encoded in the source code or the type system.
  • the C++11 std::shared_ptr because this has additional overhead for reference counting.
  • the deprecated std::auto_ptr because it has confused semantics: copying behaves like moving, which also requires that the copy source is not const so that the moved pointer can be erased from the source. (Also note that the copy assignment operator for std::vector expects a const reference, so vector<auto_ptr<T>> is non-copyable.)
  • hand-written Pimpl wrappers, since it can be challenging to implement a type correctly – I've seen enough memory leaks and segfaults from less-than-correct implementations (tip: make it impossible to use a null pointer as the impl). In fact, I recommend Pimpls to be implemented in terms of a private unique_ptr field since that takes care of all necessary resource management.

However, using a Pimpl as Bridge Pattern can enable you to design a polymorphic class hierarchy with a value-based API but pointer-like semantics. In particular, you could use vector<BaseClass> as a more convenient notation for vector<unique_ptr<BaseClassIf>>.

The “problem” with std::unique_ptr is that it requires C++11. The core idea of this pointer type is that it can't be copied, but it can be moved. Therefore, ownership is always clearly defined. When you return an object by value, it will be subject to copy semantics (even if the actual copy might be optimized away), except in C++11 where move semantics will allow you to return an unique_ptr by value.

In general, using a container of smart pointers is perfectly fine, and better than the alternatives. In your case, it fails because of the restricted semantics of C++03 with regards to auto_ptr. If you can't upgrade to C++11 (which is supported in all current mainstream compilers), you have two realistic choices: use raw pointers, or wrap the pointer in a custom Pimpl. I'd use the Pimpl if the effort is justifiable. It isn't tremendously complicated code, but you have to be careful to forward all necessary operations:

class BaseClassIf {
public:
  virtual ~BaseClassIf() {}
  virtual void someOperation() = 0;
  virtual BaseClassIf* copy() = 0; // { return new T(*this); }
}

class BaseClass {
  BaseClassIf* impl;
  void assertInvariant() { if (!impl) throw ...; }
public:
  BaseClass(BaseClassIf* impl) : impl(impl) { assertInvariant(); }

  ~BaseClass() { if (impl) delete impl; impl = 0; }

  BaseClass(const BaseClass& o) : impl(o.impl->copy()) { assertInvariant(); }

  void someOperation() { impl->someOperation(); }

private:
  BaseClass& operator=(const BaseClass&);
  // Cannot be reasonably overloaded as a value-based copy,
  // e.g. "*impl = *rhs.impl" since the exact types are unknown.
  // Cannot be overloaded as a reference copy,
  // e.g. "impl = rhs.impl" since that violates pointer ownership.
};

Note that the interface must make provisions to access the copy constructor, since the exact type is unknown by the BaseClass wrapper. This is an occasionally useful technique (e.g. as “type erasure” to hide template parameters), but here it's just annoying fallout from using polymorphism. Also note that it is not possible to define a copy assignment operator for the adapter, unless you include a common virtual BaseClassIf::operator=(const BaseClassIf&) method in your interface – but most object hierarchies cannot do an useful copy from their common base.

  • The answer to almost all pointer problems is std::unique_ptr hear hear! – stijn Dec 15 '15 at 20:33
  • Ok, I'm working now with C++11 and std::shared_ptr, works like a charm. I also did a test with unique_ptr, but it didn't compile (cannot be returned by the factory or stored in a vector?) Nevermind. – lugge86 Dec 16 '15 at 7:37
  • @lugge86 That surprises me, it should be possible to return an unique_ptr, and move-construct a new object from that return value. There's probably a way to solve your problem, but not without seeing some code. Consider asking a question on Stack Overflow and dropping the link here. – amon Dec 16 '15 at 8:24
  • I would say the answer to a lot of pointer problems is &, but great answer anyway. – Nathan Cooper Dec 19 '15 at 0:28
  • @NathanCooper references are great for certain use cases, but are full of subtle gotchas regarding value lifetime, copying, and templates when not just used for function arguments. References are likely a solution when borrowing a value without transferring ownership, but smart pointers explicitly encode who currently owns the value. – amon Dec 19 '15 at 7:37
5

To your first question: "is it good style to let a factory return method a container?":

IMHO there is nothing special with a type like std::vector<AbstractBaseClass*> - it is, at least in principle, a user defined type like any other type. You could make a typedef alias like

 typedef std::vector<AbstractBaseClass*> MyAbstractBaseClassContainer;

if that gives you a better abstraction, or wrap it within a new class. The latter gives you the opportunity to make the constructor private, so prohibiting the creation of objects of that type by any different means than a static class member factory method. But that is actually not necessarily required for a "good" factory - if std::vector<AbstractBaseClass*> suits your needs, go ahead!

If you should prefer some_smart_ptr<AbstractBaseClass> over AbstractBaseClass*, especially in the context of a vector is a completely independent question from the first one. Since there were already good answers to that question, for example here or here, or @amon's answer, I am not trying to answer this one, I doubt my answer would become better.

  • OK, sure, I can typedef my container for improved readability, furthermore, when using a typedef it seems the factory is only returning one object. However, all examples I've found are showing the client code calling 10 times the factory when they need 10 objects. I call the factory jsut once and get all the required objects. Thus, my question. However, my client code does not parse the config file, thus, does not know how many objects needed. Thats the reason for my approach. – lugge86 Dec 16 '15 at 7:33
  • @lugge86: my point is: std::vector<AbstractBaseClass*> is one object, it does not just look like one. It may be a complex object, consisting of lots of parts, but that is why a factory makes actually sense. – Doc Brown Dec 16 '15 at 8:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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