GEODE-10098: Removed explicit new and delete in favor of unique_ptr #943
Conversation
cppcache/src/util/make_unique.hpp
Outdated
| #include <utility> | ||
|
|
||
| // N3656 standard revision of make_unique. | ||
|
|
There was a problem hiding this comment.
Should we put this in a namespace?
There was a problem hiding this comment.
Have a look at how it was done in this other pending PR: https://github.com/apache/geode-native/pull/891/files
Might be good to ping @gaussianrecurrence and coordinate before there's a conflict, or two definitions in the code base(!).
There was a problem hiding this comment.
There is an internal shared library, that used go be static linked into the main library, but is still linked into all the test libraries. It was intended to be a place to put common utils that both the main lib and test code could use without exporting them as symbols from the main lib. It is the perfect place to consolidate make_unique for both the main lib and tests. The two files currently in this library are obsolete and should be deleted. It would also need to be added back as a library dependency to _apache-geode library.
There was a problem hiding this comment.
When I say "internal shared library" I don't me it is compile as a shared library. It is compiled static and linked static.
There was a problem hiding this comment.
I'm going to update the PR to address the rest and work on this one tomorrow.
| std::for_each(std::begin(pools), std::end(pools), | ||
| std::bind(&PoolXmlCreation::create, | ||
| std::bind(&std::shared_ptr<PoolXmlCreation>::get, | ||
| std::placeholders::_1))); |
There was a problem hiding this comment.
Hmm... Does this modern C++ style make this any more readable?
Would dereferencing the shared_ptrs as local variables prior to these calls help with readability?
Would a lambda rather than std::bind read easier?
Without the side by side diff I honestly can't say I understood what is going on here without unwinding all the algorithm magic going on here.
| cacheCreation_->setPdxReadSerialized(false); | ||
| } | ||
| } | ||
| CacheRegionHelper::getCacheImpl(cache_)->setPdxIgnoreUnreadFields( |
There was a problem hiding this comment.
Are you certain this logic does not override the default value of property. The original code only change the value if the values was provided in the XML, preserving the default value in the object.
I also got rid of an auto-pointer type class that could be replaced with unique_ptr.