#PIMPL naming convention

71 messages · Page 1 of 1 (latest)

keen peak
#

Is there a compelling reason to adopt a convention for the name of the internal pointer of a PIMPL type?

class FooPrivate;

class Foo
{
public:
  // public interface goes here
private:
  std::unique_ptr<FooPrivate> pimpl_;
};

I ask because in a code review the reviewer suggested that pimpl_ is not very descriptive. I think this reviewer has never seen or worked with a PIMPL before.
Mine is one of those "odd" employers that wants to be very verbose in their naming schemes. That explains the reviewer's comment.

Yet I feel renaming this variable to something like fooImplementation_ may actually hurt efforts here.
Any article I find on the internet that describes the PIMPL uses similar parlance.
Most favour pimpl or slight variations thereof.

Fretting about nothing?
Or is there some merit to a convention, no matter how tenuous?

obsidian islandBOT
#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

steel pond
#

Not what you asked, but I'd make FooPrivate a member class

feral parcel
keen peak
fading dove
#

personally, I apply a very simple convention: don't have pimpls, then you won't have to name them 😛

#

I've never found pimpl to be a compelling design. quite the opposite, in fact.

#

ime, you either have performance requirements, in which case you do not want pimpls, or you don't, in which case a normal interface is a much simpler and cleaner solution. I don't think I've ever seen a case where the potential, microscopic performance advantage of pimpl over an interface would have done anything that's worth the mess pimpl makes.

vocal sigil
#

but my ABI stability and compilation firewall nooo

fading dove
small fox
#

wrt ABI and internal migration pimpl just sounds simpler

#

you actually get to break things internally with the pimpl layer on top

fading dove
small fox
#

well adding thing at the back is a way of saying adding virtual function breaks abi

#

if you want to add virtual functions or change the setup of virtual functions in your internal implementation, if you're using a "public interface", you just cannot

steel pond
#

I'd use PIMPL over interfaces because wanting the build speed benefits is an implementation detail, and by using an interface you're leaking this to the user. But bing_shrug

small fox
#

to make a somewhat dumb example say you start off with

class interface_v1
{
public:
  virtual void FooAndBar() = 0;
  virtual ~interface_v1() = default;
};

or some such, and initially you don't have any use cases for splitting doing just foo and doing just bar, but later on you realize that it's useful for your implementation in a bunch of cases or just for refactoring, and you want to switch to

class interface_v2
{
public:
  virtual void Foo() = 0;
  virtual void Bar() = 0;
  inline void FooAndBar() { Foo(); Bar(); }
  virtual ~interface_v2() = default;
};

then you've completely broken your abi

#

FooAndBar -> FooThenBar but whatever

#

if you have a pimpl or nvi or whatever, it doesn't matter for previously compiled code that linked against FooAndBar

fading dove
#

yeah but it's also trivial to avoid this ABI break

small fox
#

as long as you still export FooAndBar on the pimpl

small fox
#

evolving internals are a thing

fading dove
#

by not adding the new methods in front of the old ones but behind

small fox
#

the only to avoid the break is to make foo and bar non virtual

small fox
fading dove
#

not on any system I'm familiar with

small fox
#

actually I need to recheck that

#

but even if you can avoid that, it's just plain annoying to deal with

vocal sigil
# 
class interface_v2 : public interface_v1 { ... };

is popular with microsoft trolol

fading dove
#

that's likely a better way to do it yeah

small fox
#

I'd rather not deal with any of that tbh

fading dove
#

then you've even documented the different versions of the interface

small fox
#

but fair enough, there are cases where you can avoid it

vocal sigil
#

let me just dynamic_cast QueryInterface for ID3D12Device15

small fox
#

please not that

#

I mean sure it works but I hate this sytem so much

vocal sigil
#

sorry, exaggerating

#

it was only ID3D12Device14

fading dove
#

I don't think it actually goes higher than like 3 or smth 😛

#

dxgi is probably the worst I've seen. they're at like 6 now iirc

vocal sigil
#

Agility SDK baby

fading dove
#

excuse me? xD

#

lol

vocal sigil
#

D3D12 Ultimate or whatever

fading dove
#

fun

small fox
#

from a less technical pov I'll still handle the polymorphic pointer myself anytime of the day rather than let users do it... ran into another wonderful "are you kidding me moment" not too long ago

#

it was in the same vein as

void old_thing(std::string const* optional_read_only_string = nullptr);

//
std::string some_local_var;

std::string* copy = new std::string(some_local_var);
old_thing(copy);
delete copy;

but worse

#

those are my users and it makes me go insane

fading dove
#

wtf

#

who takes an std::string by pointer oO

small fox
#

optional wasn't a thing

fading dove
#

k ^^

small fox
#

at the time that was written

fading dove
#

wait lol, only just saw the way it was being called

#

lol wtf

small fox
#

I could have made it clearer, but "old_thing" there is a legacy piece of interface in the component I'm working with, the calling code is from users that make me go insane

fading dove
#

I mean, honestly, at that point, dunno, I think a basic level of competence can be assumed for your users. if someone writes code like that, that's not a failure of the interface, that's just completely incompetency on the user's part.

small fox
#

I hear you but it also just makes my job easier down the line if they just can't do a stupid

#

well, let's just I've had mixed success there

#

they just keep finding ways to make more stupid

fading dove
#

like, seriously, "I see pointer so I must use new" is not a thought process you should find in anyone who has been using C++ for more than approximately three hours.

small fox
#

assuming you are correct, then I must investigate the other department, they've clearly invented a hyperbolic time chamber and I want in

#

I just reread myself and I guess it's the other way around, so elliptic time chamber? ||no one read the typo before correction||

#

I'm confused and hurt myself in my confusion

fading dove
#

anyways, if backwards ABI compatibility is smth you actually need then imo it's better to actually properly version your interfaces than to implicitly rely on the new build being ABI compatible with all prior builds.

small fox
#

for abi concerns, I don't consider having a public interface type for public use "nicer" for abi purposes

keen peak
#

!solved