PIMPL pattern and Shared about farm-ng-core HOT 4 OPEN

Well, I would generally recommend this pattern in favor of traditional PIMPL in our codebase unless there is a good reason.

👍

As for when to use any implementation hiding at all, I would personally recommend to do so for large components with complex dependencies or when you think there may be more than one implementation of a thing.

OK, that makes sense and seems like a good guideline to me. I think maybe in some areas of the codebase we assumed this was a conventional/necessary part of the Component pattern, when in fact it was just a design choice you happened to make for specific components.

For recommendation #1 above I can summarize this thread and open a PR against the style guide.

For recommendations #2 and 3 above I can prototype in the form of a PR and see whether we like it. Not sure on what timeline though 😅.

from farm-ng-core.

I agree with your statements, Ian. The not_null type wrapper looks cool!

In case it's useful, here is my general summary which I think agrees with your outline:

• Faster compilation
• Minimize boilerplate (compared to PIMPL)
• Code readability (compared to PIMPL)
• Explicit factory instantiation (as opposed to implicit in each PIMPL value type)
• Extensible - PIMPL types are concrete, so code that takes them are not working to an interface. To use a custom implementation of the type, you must modify the concrete PIMPL implementation which you may or may not own. You cannot use an external factory method. You could solve this by deriving your PIMPL type from an interface, but then you yet more indirection - you may as well just use the interface directly.
• Simple casting (Down / Up) for concrete type access when needed. (This is probably more controversial, but there are times that an implementation needs access to the concrete type. For example, two GL backend PIMPL objects using one another. Or a VideoDevice that may optionally support a Seekable interface or something.)

Disadvantages of Interface + Factory over traditional PIMPL

• I hadn't considered the ABI issues that your link mentions but I don't believe we have any examples where we have been trying to do that. We have generally been using proto + GRPC for stable interchange.
• Function inlining - there may be cases where the concrete forwarding in traditional PIMPL can inline a call, whereas a virtual dispatch is necessary with the interface approach. That said, if the PIMPL implementation is in its own compilation unit as it should be, the call into the PIMPL cannot be inlined, right? To fix that, whole program optimization would have to perform the inlining, but that could also potentially inline the virtual call.

from farm-ng-core.

What are your thoughts on when the pattern should be used @stevenlovegrove?

Outside of situations where we clearly need a compilation firewall (e.g. to avoid pulling in a large header dependency), I see it as having two benefits:

• Be proactively defensive against large compilation times as the codebase grows
• The extensibility case you make above; which should significantly improve testability. Especially if our APIs evolve in such a way that dependencies are taken as interfaces.

But I don't have a good instinct on a guideline for when it should be applied. Any thoughts?

from farm-ng-core.

Well, I would generally recommend this pattern in favor of traditional PIMPL in our codebase unless there is a good reason.

As for when to use any implementation hiding at all, I would personally recommend to do so for large components with complex dependencies or when you think there may be more than one implementation of a thing. I'm not generally too prescriptive though, so whenever it makes things simpler and not more complicated. What are your thoughts?

from farm-ng-core.