Correct pattern for nested private class that inherit from outer class

Question

I am trying to implement a pattern in C++ where a nested private class inherits from the outer class and the private class is instantiated via a static factory method in the outer abstract class.

I have this code now, that compiles, but I am not sure whether I did it correctly.

Search.h:

namespace ns_4thex {
    class Search {
        private:
            class Implementation;
        public:
            static Search & create();
            virtual int doIt() = 0;
    };
    class Search::Implementation: public Search {
        int doIt();
    };
}

Search.cpp:

#include "Search.h"
using namespace ns_4thex;
Search & Search::create() {
    return *(new Search::Implementation());
}

int Search::Implementation::doIt() {
    return 0;
}

Thought?

pretty sure that `return *(new Search::Implementation());` will cause a ton of memory leaks — Alberto Sinigaglia, Jun 04 '21 at 00:24
@AlbertoSinigaglia Isn't it going to be the responsibility of the consumer to call delete on the reference when it is no longer needed? — 4thex, Jun 04 '21 at 00:26
This looks like you're trying to create the `pimpl` pattern, and also need to learn [how to use singletons in C++](https://stackoverflow.com/questions/1005685/c-static-initialization-order) — Mooing Duck, Jun 04 '21 at 00:28
@MooingDuck Thank you for the useful pointer. Yeah, the pimpl patterns seems like an interesting study. I used to program in C++ like 20 years ago, and now I decided to get certified. — 4thex, Jun 04 '21 at 00:37

score 1 · Answer 1 · answered Jun 04 '21 at 01:50

1

Your example has potentially a memory leak. The factory pattern should return a pointer type instead of the reference type. The caller using it can free the allocated memory

Search* Search::create() {
    return new Search::Implementation();
}

answered Jun 04 '21 at 01:50

nhatnq

1,173
7
16

2

It would better to use `make_unique` and return `unique_ptr` to make the ownership clean. – prehistoricpenguin Jun 04 '21 at 01:51
So you cannot call delete on a reference using the address of operator (&)? – 4thex Jun 04 '21 at 15:30
@prehistoricpenguin If you make your comment an answer, I will make it the accepted answer to the question. – 4thex Jun 05 '21 at 12:00
@4thex The pattern in your code seems interesting, this is the first time I have seen this design, could you please share some use case scenarios? – prehistoricpenguin Jun 05 '21 at 12:20
1

@prehistoricpenguin It is a pattern I have used in C#. Instead of using new and be tempted to pass the implementation around as arguments, I wanted to basically make that impossible. Only the abstract type will be passed around, and the only way to create an instance would be via the factory method. Alternative implementations can then be easily created later if needed. – 4thex Jun 07 '21 at 16:11

score 1 · Accepted Answer · answered Jun 05 '21 at 12:18

A static factory method always returns a pointer type. So the create function should return a pointer or smart pointers in modern c++.

The declaration:

static std::unique_ptr<Search> create();

The definition:

std::unique_ptr<Search> Search::create() {
  return std::make_unique<Search::Implementation>();
}

The complete code may like this:

#include <memory>
namespace ns_4thex {
class Search {
 private:
  class Implementation;

 public:
  static std::unique_ptr<Search> create();
  virtual int doIt() = 0;
};

class Search::Implementation : public Search {
  int doIt();
};

std::unique_ptr<Search> Search::create() {
  return std::make_unique<Search::Implementation>();
}

int Search::Implementation::doIt() { return 0; }
}  // namespace ns_4thex

Correct pattern for nested private class that inherit from outer class

2 Answers2