8

I have something like the following:

// foo.h:
class foo {
public:
    foo(); 
    ~foo();
    // note: the param type repetition here is only incidental, assume the
    // functions can't easily be made to share type signatures
    void bar(a b, c d);               
    void baz(a b, c d, e f, g h, i j);
    void quux(a b, c d, e f, g h, i j);
private:
    class impl;
    impl *m_pimpl;
}

Then:

// foo.cpp:
class foo::impl {
public:
    void bar(a b, c d);
    void baz(a b, c d, e f, g h, i j);
    void quux(a b, c d, e f, g h, i j);

private:
    // lots of private state, helper functions, etc. 
};

void foo::impl::bar(a b, c d) { ... }
void foo::impl::baz(a b, c d, e f, g h, i j) { ... }
void foo::impl::quux(a b, c d, e f, g h, i j) { ... }

foo::foo() : m_pimpl(new impl()) { }
foo::~foo() { delete m_pimpl; m_pimpl = NULL; }
void foo::bar(a b, c d) {
    return m_pimpl->bar(b, d);
}
void foo::baz(a b, c d, e f, g h, i j) {
    return m_pimpl->baz(b, d, f, h, j)
}
void foo::quux(a b, c d, e f, g h, i j) {
    return m_pimpl->quux(b, d, f, h, j);
}

There's a lot of repetition of bar, baz, and quux here:

  • Once in foo's declaration
  • Once in foo::impl's declaration
  • Once in foo::impl's definitions
  • Twice in foo's definitions: once for foo's function param list and again to call the corresponding foo::impl functions.

For each of these except the last I have to write out the entire parameter list, whose types can be more involved.

What's the best way, if any, to reduce the repetition here? One easy one is to inline the definition of the foo::impl public functions, but is there anything besides that outside of designing the classes differently to have fewer public functions?

Claudiu
  • 206,738
  • 150
  • 445
  • 651
  • Define a type signature, and use it instead of listing arguments in a few of those spots? Maybe use decltype and some traits to help? – Yakk - Adam Nevraumont Apr 10 '15 at 22:57
  • @Yakk: Hmm could you give an example? – Claudiu Apr 10 '15 at 23:00
  • 1
    For dealing with the boilerplate constructor and destructor, see http://stackoverflow.com/q/8595471/103167 – Ben Voigt Apr 10 '15 at 23:06
  • 1
    Wouldn't it be nice if the assembly code would just replace `ECX` to `m_pimpl` and `jmp` to appropriate function... – Dialecticus Apr 10 '15 at 23:16
  • 1
    I tend to implement the impl class's methods in the class definition, which reduces some of the repetition. – Adrian McCarthy Apr 10 '15 at 23:35
  • @AdrianMcCarthy: That makes sense. They were already implemented though and I didn't want to have to figure out everywhere I had to stick a `m_pimpl->`. That is an option though, that'll reduce it by a good amount. – Claudiu Apr 10 '15 at 23:40

4 Answers4

4

Using signatures, we can reduce it to 3 mentions, plus 1 set of forwards:

using signature_1 = int(int);

struct foo {
  signature_1 x;
  foo();
  ~foo();
private:
  struct fooimpl;
  std::unique_ptr<fooimpl> pimpl;
};

int main() {
  foo f;
  std::cout << f.x(1) << '\n';
}
struct foo::fooimpl {
  signature_1 x;
  int v = 3;
};

foo::~foo() = default;
foo::foo():pimpl(new foo::fooimpl()){}

int foo::x(int y){return pimpl->x(y);}
int foo::fooimpl::x(int y){return y+v;}

We can get it down to 2+forward if our pimpl is a pure virtual class. Write a map from decltype(&foo::method)->signature, and have the pure virtual interface use that (and decltype) to invent the signature of the pure virtual class.

Write the signature once in foo, decltype-determine it in foo_impl_interface, and then implement it inline within foo_impl in the cpp file. Plus one set of forwards.

 template<class MethodPtrType>
 struct method_sig;
 template<class MethodPtrType>
 using method_sig_t = typename method_sig<MethodPtrType>::type;
 template<class T, class R, class...Args>
 struct method_sig< R(T::*)(Args...) > {
   using type=R(Args...);
 };

plus another 11 odd specializations (sigh) to get all the cases. (const& const const&& const volatile const volatile& const volatile&& & && volatile volatile& volatile&& qualifiers are all, as far as I can tell, required).

Now method_sig_t<decltype(&foo::x)> is int(int), which we can use:

struct foo_impl_interface {
  virtual method_sig_t<decltype(&foo::x)> x = 0;
  virtual ~foo_impl_interface() {}
};

assuming we are using pimpl to regularize our type rather than hide the state.


Finally, instead of implementing most of your code in the pimpl, instead just store STATE in the pimpl, leave the code in the class itself.

This gives you the "I others do not depend on my size": who cares if the code is in foo or foo_impl that reads foo_impl's state? So if you aren't doing the foo_impl_interface technique, why forward?

Yakk - Adam Nevraumont
  • 235,777
  • 25
  • 285
  • 465
  • Interesting, I'll have to take a close look. About the last question: I'd also want all of the private helper functions in the `impl`, no? – Claudiu Apr 10 '15 at 23:48
  • @Claudiu sure, but those aren't creating forwarding boilerplate. – Yakk - Adam Nevraumont Apr 10 '15 at 23:49
  • If your pimpl derives from an interface known in the header file, you could use a smart pointer instead of pimpl. Maybe add resource management functions on top of it, like copying and non-empty default construction; but the user could work with the interface directly via `operator->`. – dyp Apr 11 '15 at 00:02
  • @dyp As an example of this pattern, `std::function`: implementations not only expose the pimpl interface, but the pimpl implementation in the header file! Should `std::function` have exposed its pimpl publicly, and implemented the non-call non-copy operations via `->`? – Yakk - Adam Nevraumont Apr 11 '15 at 00:07
  • Not quite sure I fully understand what you mean, and not quite sure which operations you're talking about. I would guess `std::function` needs to "expose" its pimpl impl in the header because the call signature is dependent. It also doesn't have many operations that don't involve the pointer directly (`target` and `target_type` maybe?). – dyp Apr 11 '15 at 00:14
  • @dyp `target` and `target_type` are both going to be basically forwards to the pimpl. `swap` `assign` `operator=` etc cannot be, as they will change the pimpl. `operator()` cannot be `->()` obviously, and having it "look like" a function is useful. So, `target` and `target_type`. `shared_ptr` also does some erasure, but its `->` is taken. But that does bring up an advantage: your method forces vtable based erasure, while if you hide it you can swap the vtable erasure for a manual erasure without impacting the interface (except ABI). Also, if you want to mimic say iterator concept. – Yakk - Adam Nevraumont Apr 11 '15 at 00:20
  • I don't think "my" method (looks like the same as in [Galik's answer](http://stackoverflow.com/a/29572523/)) *forces* vtable-based erasure. For example, you can hack it by returning `this`, making `->` essentially equivalent to `.` – dyp Apr 11 '15 at 00:23
  • @Yakk s/11/23/. You forgot `R(T::*)(Args..., ...)`. The number will go up to 48 with [`transaction_safe`](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4301.pdf), and 96 if [N4320](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4320.html) gets adopted. – T.C. Apr 11 '15 at 00:31
  • @T.C. I should have stuck with 20-odd (my original guess) rather than counting. :/ Can we deduce the raw member function pointer without modifiers in a function template pattern matching context? That would defeat the combinatorial explosion with some fancy decltypes I think. – Yakk - Adam Nevraumont Apr 11 '15 at 01:39
  • @Yakk Apparently yes. Both the TM TS and N4320 add a special case to the function call deduction rules to allow deduction with a conversion. – T.C. Apr 12 '15 at 22:59
2

If your public class just proxies calls to the implementation, consider using interface -> implementation model instead of pimpl.

You would reference your implementation only by the interface in the user code with class foo being interface

class foo {
public:
    virtual void bar(a b, c d) = 0;               
    virtual void baz(a b, c d, e f, g h, i j) = 0;
    virtual void quux(a b, c d, e f, g h, i j) = 0;
    virtual ~foo(){}
}

Idea behind pimpl is to store private data and functions in the separate object pointer so you don't break your public class interface in case of data storage and handling change. But it does not imply all code moved to the private object. So you usually implement your public functions just in place. You wouldn't break user code when your public functions implementation change, as your public functions implementation will be hiden from the user along with private class definition.

Lol4t0
  • 12,018
  • 4
  • 26
  • 60
  • Wouldn't you now need a factory? The pimpl approach lets clients just instantiate a foo as an automatic (local) variable. Sure, the implementation is still in the freestore, but that's an implementation detail that's hidden from the client. With the virtual interface, the client needs to call a factory function to explicitly instantiate the object. – Adrian McCarthy Apr 10 '15 at 23:55
  • @AdrianMcCarthy of course it's true. Note however that I also propose another solution similar to your own comment to the question as another alternative. Sometimes you prefer one and sometimes another. – Lol4t0 Apr 11 '15 at 00:08
  • Ah that's good point about the factory. At first this solution seemed ideal but then I hadn't realized that creators of `foo`s would have to import `foo_impl.h` and call a different constructor (or something of the sort). – Claudiu Apr 11 '15 at 00:31
0

One way you can do this is by defining an interface class so you don't need to redeclare everything and use pass-through pointer semantics (overloaded operator->)

Here is an example: Is it possible to write an agile Pimpl in c++?

Community
  • 1
  • 1
Galik
  • 42,526
  • 3
  • 76
  • 100
0

To close out this question: ultimately I think Adrian's comment addresses the issue the best: "I tend to implement the impl class's methods in the class definition, which reduces some of the repetition."

My code above would become:

// foo.cpp:
class foo::impl {
public:
    // lots of private state, helper functions, etc. 
};

foo::foo() : m_pimpl(new impl()) { }
foo::~foo() { delete m_pimpl; m_pimpl = NULL; }
void foo::bar(a b, c d) {
    ... code using m_pimpl-> when necessary ...
}
void foo::baz(a b, c d, e f, g h, i j) {
    ... code using m_pimpl-> when necessary ...
}
void foo::quux(a b, c d, e f, g h, i j) {
    ... code using m_pimpl-> when necessary ...
}

Now it's much more reasonable - just one for the declaration and one for the definition. There's the small overhead when converting a class to use pimpl of adding the m_pimpl->s, but IMO this is less annoying than having all the repetition.

Community
  • 1
  • 1
Claudiu
  • 206,738
  • 150
  • 445
  • 651