Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using std::shared_ptr instead of raw pointers in usSharedData.h #78

Closed
jeffdiclemente opened this issue Feb 25, 2016 · 3 comments

Comments

@jeffdiclemente
Copy link
Member

std::shared_ptr can now be used for all instances where SharedData, SharedDataPointer, and ExplicitlySharedDataPointer were used.
Doing this will add robustness to the code base and make maintenance easier.

@saschazelzer
Copy link
Member

I am not sure I understand this. The mentioned classes implemented the COW pattern (copy on write). SharedDataPointer does so implicitly (when calling non-const methods on an object with ref count > 1) and ExplicitlySharedDataPointer only detaches a shared instance on explicit request. How can std::shared_ptr be used instead while keeping this functionality?

@jeffdiclemente jeffdiclemente changed the title Remove usSharedData.h and replace with usage of std::shared_ptr Consider using std::shared_ptr instead of raw pointers in usSharedData.h Mar 8, 2016
@jeffdiclemente
Copy link
Member Author

It helps if I read the code a little more closely...

You're right, the SharedDataPointer and ExplicitlySharedDataPointer can't be completely replaced with std::shared_ptr since they are used for copy-on-write.

After looking again, I can only say that perhaps we should consider using std::shared_ptr instead of a raw pointer within SharedDataPointer and ExplicitlySharedDataPointer to store the shared data object.

Additionally, I think there is a bug in usSharedData.h on line 75:
inline const T* Data() const { return d; }.
Shouldn't this be inline T* Data() const { return d; }?

@jeffdiclemente
Copy link
Member Author

no longer relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants