pdxcodemonkey commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r441056111
##########
File path: cppcache/src/PdxType.cpp
##########
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr<PdxType> otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+ return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+ return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash<std::string> strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector<std::shared_ptr<PdxFieldType>>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+ auto pdxPtr = *it;
+ result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
One thing that is essential we get right here is that ordering of fields
should have no bearing on the generated hash. so { "foo": 7, "bar" 10 } is the
same type as { "bar": 12, "foo": 11 }, e.g. Otherwise we end up with a "type
explosion" of zillions of copies of the same thing each getting their own type
ID. If we're lucky, the server side will sort the field names when generating
the type ID on that side, and they'll all get the same ID, but then we've still
made a ton of unnecessary trips to the server.
##########
File path: cppcache/src/PdxType.cpp
##########
@@ -557,11 +557,30 @@ bool PdxType::Equals(std::shared_ptr<PdxType> otherObj) {
}
bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
+ if (m_geodeTypeId < other.m_geodeTypeId) {
+ return true;
+ }
+
+ if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
+ return this->m_className < other.m_className;
+ }
+
+ return false;
+}
+
+int32_t PdxType::hashcode() const {
+ std::hash<std::string> strHash;
+ auto result=strHash(this->m_className);
+
+ for (std::vector<std::shared_ptr<PdxFieldType>>::iterator it =
+ m_pdxFieldTypes->begin();
+ it != m_pdxFieldTypes->end(); ++it) {
+ auto pdxPtr = *it;
+ result = result ^ (strHash(pdxPtr->getClassName()) << 1 );
Review comment:
Do we need to xor in the classname each time through the loop? It seems
like just using the field names in here would be more appropriate
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
