Design and documentation

[Laurent]

I already thought about using pimpl.

It would help for:
- binary backward compatibility
- hiding all the ugly private details (which is the main purpose of the technique)
- allowing easy ref-counted sharing and COW if I need it (like for resources classes)
- removing the need for const_cast or mutable, indeed; but only with an implementation that doesn't forward constness to the private pointer

I recently implemented pimpl_ptr classes at work, and things can be encapsulated enough so that it is safe enough to use this idiom.

I'm still not sure whether I should use it or not, though.

[nullsquared]

You need to implement your own copy-constructor and operator.

Yes.  I don't understand how that is an issue?  Don't avoid features of the language for no reason.

but only with an implementation that doesn't forward constness to the private pointer

What do you mean?  The pointer itself is const, as in that it cannot change.  However, it is not a const pointer to a const instance, which means you can modify whatever it points to.

I recently implemented pimpl_ptr classes at work, and things can be encapsulated enough so that it is safe enough to use this idiom.

What's a pimpl_ptr?  A shared_ptr works fine...?

[Dravere]

Yes.  I don't understand how that is an issue?  Don't avoid features of the language for no reason.

It is not a feature of the language, it is a workaround to reduce dependencies and/or hide private details, which the language can't do by itself.

If you use it to avoid mutable or const_cast it is even more a hack than a solution. Don't forget that pimpl also adds several overhead. You need additional indirection, lose inlining and have additional heap allocation. Especially with small objects this can be a problem, because the heap often isn't optimized for small object but for big ones.

What do you mean?  The pointer itself is const, as in that it cannot change.  However, it is not a const pointer to a const instance, which means you can modify whatever it points to.

Exaclty and this is what he meant. You can change data even the object should be const. So you lose the security at compile time.

What's a pimpl_ptr?  A shared_ptr works fine...?

A pimpl_ptr probably is a class, that automaticly creates a new instance when it is copied. So you don't need to implement your own copy-constructor or operator=.

Dravere

[Laurent]

What do you mean? The pointer itself is const, as in that it cannot change. However, it is not a const pointer to a const instance, which means you can modify whatever it points to.

You can wrap your private pointer into a class that forwards constness, like this:

Code: [Select]

template <typename T>
class PimplPtr
{
public :

    T& operator *();
    const T& operator *() const;

    T* operator ->();
    const T* operator ->() const;

   ...
};

class Class
{
public :

    void ConstFunc() const
    {
        myData->some_member = 5; // error, operator -> forwards constness
    }

private :

    PimplPtr<ClassImpl> myData;
};

Doing so is a good idea, as it makes the use of an extra indirection completely transparent, you don't loose constness on the private members.
Which leads to the same conclusion as Dravere: using pimpl to get rid of constness is purely a hack, a good implementation should forward it like above.

Don't forget that pimpl also adds several overhead. You need additional indirection, lose inlining and have additional heap allocation. Especially with small objects this can be a problem, because the heap often isn't optimized for small object but for big ones.

Yup, that's why I don't use it at the moment.

A pimpl_ptr probably is a class, that automaticly creates a new instance when it is copied. So you don't need to implement your own copy-constructor or operator=.

Exactly. Just like shared_ptr or scoped_ptr, we can create a pimpl_ptr class that gives the same semantics as a raw pointer but with correct copy and assignment.

[nullsquared]

It is not a feature of the language, it is a workaround to reduce dependencies and/or hide private details, which the language can't do by itself.

Constructors and operator=() aren't part of the language?

If you use it to avoid mutable or const_cast it is even more a hack than a solution.

No it's not.  It's a different design.

Don't forget that pimpl also adds several overhead.

Come back with some profiler results and I'll consider your argument.

Exaclty and this is what he meant. You can change data even the object should be const. So you lose the security at compile time.

C++ const objects are bitwise const, not logical const.  Security of what is lost?  We're talking about mutable members.

[blewisjr]

shared_ptrs are excellent to use however, there are reasons why a lot of game devs including the pros avoid them.  No they don't care about the double heap allocations that boost does.  They care about that just typing #include<boost/shared_ptr.hpp> increases the compile time of a already 3 day long compiled application.  That is not even the main reason. A lot of game dev's avoid shared pointers because it does not allow them to explicitly control the life time of their heap allocated objects.

When you are creating resources and destroying resources at 60fps for say particle effects, maybe arrows that fire from a bow or bullets flying from a gun shared_ptrs are death to the code base because the objects linger longer then the should.

There is a whole article about it in general.  At the end they give some reflections on shared_ptr and games.

http://www.gamasutra.com/view/feature/4015/managing_data_relationships.php

On another note Laurent did you look at Intrusive Pointers?
http://www.boost.org/doc/libs/1_41_0/libs/smart_ptr/intrusive_ptr.html

[nullsquared]

They care about that just typing #include<boost/shared_ptr.hpp> increases the compile time of a already 3 day long compiled application.

That's true, however, shared_ptr doesn't make that much of a slowdown.  I'd worry more about the preprocessor library.

A lot of game dev's avoid shared pointers because it does not allow them to explicitly control the life time of their heap allocated objects.

somePtr.reset(); // magic

When you are creating resources and destroying resources at 60fps for say particle effects, maybe arrows that fire from a bow or bullets flying from a gun shared_ptrs are death to the code base because the objects linger long then the should linger.

theContainer.clear(); // magic

There is a whole article about it in general.  At the end they give some reflections on shared_ptr and games.

http://www.gamasutra.com/view/feature/4015/managing_data_relationships.php

The whole point of shared_ptr is for ... shared objects.  Not just memory management in general.

[blewisjr]

You should know even reset on the shared_ptr and clear on the container are not guaranteed to free the memory.  Because if the ref count is not 0 the memory will remain allocated when you call reset().

[edit]
The point being those calls are not 100% reliable.  Like I said shared_ptr's are great to use but they defiantly should not be over used.  They are not the end all be all.

[nullsquared]

You should know even reset on the shared_ptr and clear on the container are not guaranteed to free the memory.  Because if the ref count is not 0 the memory will remain allocated when you call reset().

[edit]
The point being those calls are not 100% reliable.  Like I said shared_ptr's are great to use but they defiantly should not be over used.  They are not the end all be all.

The whole point of shared_ptr is for ... shared objects.  Not just memory management in general.

If you don't want to share the memory, then don't.  shared_ptr's don't just sneak in some references out of no where so you can't free the memory...

[Dravere]

Constructors and operator=() aren't part of the language?

I didn't say anything against copy-constructor or operator=(), so I didn't understand your argument there. That's why I thought you meant pimpl.

I only said that there is an additional possibility of failures, that was my point and nothing more.

No it's not.  It's a different design.

Ok, it is a hack design or a bad design

Come back with some profiler results and I'll consider your argument.

This response is simply ... I don't know the best word for it in english ... perhaps it is better that way :|

You can also think a bit about performance without using a profiler. It is a fact that the windows heap that is used through new in C++ is optimized for bigger and slow for small objects.
That is also why Andrei Alexandrescu implements a small object allocator in his book Modern C++ Design. There are also other books pointing out, that the heap in C++ is very slow for small objects and you should better consider copying them.

But well ... if you simply want to ignore facts ... go on ...

C++ const objects are bitwise const, not logical const.  Security of what is lost?  We're talking about mutable members.

No we aren't or not only. When Laurent said he thought about using pimpl we changed to the use of pimpl for the whole class. Because only that way, you can have all those things that he listed

Dravere

[blewisjr]

You should know even reset on the shared_ptr and clear on the container are not guaranteed to free the memory.  Because if the ref count is not 0 the memory will remain allocated when you call reset().

[edit]
The point being those calls are not 100% reliable.  Like I said shared_ptr's are great to use but they defiantly should not be over used.  They are not the end all be all.

The whole point of shared_ptr is for ... shared objects.  Not just memory management in general.

If you don't want to share the memory, then don't.  shared_ptr's don't just sneak in some references out of no where so you can't free the memory...

Correct if you don't share the memory.  But if you don't share the memory you should not be using shared pointers to begin with.  No point in having that extra overhead when you can just used a scoped pointer.

[nullsquared]

Come back with some profiler results and I'll consider your argument.

You can also think a bit about performance without using a profiler. It is a fact that the windows heap that is used through new in C++ is optimized for bigger and slow for small objects.

If you're so worried about your heap being slow, you shouldn't even be using the default heap allocator.  Use nedmalloc.  And, obviously, don't allocate primitive types on the heap, that's just stupid.  The whole point was to keep the mutable members in an internal structure, not all the members.  If all the members need to be mutable then I believe a major redesign is in order.

But if you don't share the memory you should not be using shared pointers to begin with.

How is that any different from my original statement??  Like I said, "The whole point of shared_ptr is for ... shared objects. Not just memory management in general."

[T.T.H.]

To add some other personal experience I can say that this...

Code: [Select]

window.SetView(sf::View(...));
...was so far one of the biggest headaches I ever had with using SFML because it did cost me so much time to find out what I did wrong. I'm pretty relieved that others did stumble about it, too, and that Laurent sees the issue.

1. Using a const reference makes really unclear what the function does with the object, and something must be done to prevent people from using it wrongly. I'll for sure improve the documentation a lot, and regarding this particular RenderTarget::SetView function, I'm thinking about handling views by value rather than by keeping references to user instances.

Personally I'm for totally for assign-by-value in this case as I don't see any benefit in doing it differently. Whenever the user wants to change the view somewhere there will be at least one assignment - why not do it right where it finally matters (inside CWindow) and in a way with the fewest possible usage errors (including no out-of-scope issue)?

I'm not going to use pointers because of personal design choices, but we all agree that it's not the only solution anyway.

Personally I don't need smart pointers, too.

Just as a sidenote: I do use Boost in combination with SFML myself but I actually do see a good benefit in SFML having no dependencies at all. No matter how easy or how complex a dependency is, it is just yet another thing one has to care about and keep it up-to-date and fix errors when updates brought breaking changes. In addition for a beginner Boost can easily look like a big, scary monster. Well, it might the most rewarding monster ever for a C++ beginner to face, but it can be scary nevertheless.