[
https://issues.apache.org/jira/browse/GEODE-10098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17503069#comment-17503069
]
ASF GitHub Bot commented on GEODE-10098:
----------------------------------------
pivotal-jbarrett commented on a change in pull request #943:
URL: https://github.com/apache/geode-native/pull/943#discussion_r821893371
##########
File path: cppcache/src/CacheXmlParser.cpp
##########
@@ -562,27 +555,15 @@ void CacheXmlParser::startCache(const xercesc::Attributes
&attrs) {
}
}
- _GEODE_NEW(cacheCreation_, CacheXmlCreation());
+ cacheCreation_ = make_unique<CacheXmlCreation>();
}
void CacheXmlParser::startPdx(const xercesc::Attributes &attrs) {
- auto ignoreUnreadFields = getOptionalAttribute(attrs, IGNORE_UNREAD_FIELDS);
- if (!ignoreUnreadFields.empty()) {
- if (equal_ignore_case(ignoreUnreadFields, "true")) {
- cacheCreation_->setPdxIgnoreUnreadField(true);
- } else {
- cacheCreation_->setPdxIgnoreUnreadField(false);
- }
- }
-
- auto pdxReadSerialized = getOptionalAttribute(attrs, READ_SERIALIZED);
- if (!pdxReadSerialized.empty()) {
- if (equal_ignore_case(pdxReadSerialized, "true")) {
- cacheCreation_->setPdxReadSerialized(true);
- } else {
- cacheCreation_->setPdxReadSerialized(false);
- }
- }
+ CacheRegionHelper::getCacheImpl(cache_)->setPdxIgnoreUnreadFields(
Review 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.
##########
File path: cppcache/src/CacheXmlCreation.cpp
##########
@@ -34,37 +35,21 @@ void
CacheXmlCreation::addPool(std::shared_ptr<PoolXmlCreation> pool) {
pools.push_back(pool);
}
-void CacheXmlCreation::create(Cache* cache) {
- m_cache = cache;
- m_cache->m_cacheImpl->setPdxIgnoreUnreadFields(m_pdxIgnoreUnreadFields);
- m_cache->m_cacheImpl->setPdxReadSerialized(m_readPdxSerialized);
+void CacheXmlCreation::create(Cache& cache) {
// Create any pools before creating any regions.
-
- for (const auto& pool : pools) {
- pool->create();
- }
-
- for (const auto& rootRegion : rootRegions) {
- rootRegion->createRoot(cache);
- }
-}
-
-void CacheXmlCreation::setPdxIgnoreUnreadField(bool ignore) {
- // m_cache->m_cacheImpl->setPdxIgnoreUnreadFields(ignore);
- m_pdxIgnoreUnreadFields = ignore;
+ std::for_each(std::begin(pools), std::end(pools),
+ std::bind(&PoolXmlCreation::create,
+ std::bind(&std::shared_ptr<PoolXmlCreation>::get,
+ std::placeholders::_1)));
Review comment:
Hmm... Does this modern C++ style make this any more readable?
Would dereferencing the `shared_ptr`s 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> TcrConnection::readMessage should not be explicitly allocating memory
> ---------------------------------------------------------------------
>
> Key: GEODE-10098
> URL: https://issues.apache.org/jira/browse/GEODE-10098
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Blake Bender
> Priority: Major
> Labels: pull-request-available
>
> This method calls new to read an array of bytes, then returns it to the
> caller, whose responsibility is to delete it (what the heck???). Even
> better, the memory is deleted in a call to TcrMessage::setData, so not even
> in the same class. If this memory was a std::vector<int8_t>, we could
> probably take advantage of move semantics and maybe even improve performance
> a bit, in addition to avoiding potential leaks and weirdness...
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
