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