pivotal-jbarrett commented on a change in pull request #619:
URL: https://github.com/apache/geode-native/pull/619#discussion_r442594400
##########
File path: cppcache/src/PdxType.cpp
##########
@@ -568,16 +568,32 @@ bool PdxType::operator<(const PdxType& other) const {
return false;
}
-int32_t PdxType::hashcode() const {
+bool PdxType::operator==(const PdxType& other) const {
+ if (this->m_className != other.m_className){
+ return false;
+ }
+
+ if(this->m_noJavaClass != other.m_noJavaClass){
+ return false;
+ }
+
+ if(this->getTotalFields() != other.getTotalFields()){
+ return false;
+ }
+
+ //TODO: Compare m_pdxFieldTypes & other.m_pdxFieldTypes;
+ return true;
+}
+
+size_t PdxType::hashcode() const {
std::hash<std::string> strHash;
- auto result=strHash(this->m_className);
+ auto result = strHash(this->m_className);
- for (std::vector<std::shared_ptr<PdxFieldType>>::iterator it =
- m_pdxFieldTypes->begin();
+ for (auto it = m_pdxFieldTypes->begin();
Review comment:
Can we use a range `for` loop here.
##########
File path: cppcache/src/PdxType.cpp
##########
@@ -568,16 +568,32 @@ bool PdxType::operator<(const PdxType& other) const {
return false;
}
-int32_t PdxType::hashcode() const {
+bool PdxType::operator==(const PdxType& other) const {
+ if (this->m_className != other.m_className){
+ return false;
+ }
+
+ if(this->m_noJavaClass != other.m_noJavaClass){
+ return false;
+ }
+
+ if(this->getTotalFields() != other.getTotalFields()){
+ return false;
+ }
+
+ //TODO: Compare m_pdxFieldTypes & other.m_pdxFieldTypes;
Review comment:
What's with the new TODO?
##########
File path: cppcache/src/PdxType.hpp
##########
@@ -205,10 +205,25 @@ class PdxType : public internal::DataSerializableInternal,
// This is for PdxType as key in std map.
bool operator<(const PdxType& other) const;
- int32_t hashcode() const;
+ bool operator==(const PdxType& other) const;
+
+ size_t hashcode() const;
};
} // namespace client
} // namespace geode
} // namespace apache
+namespace std {
+
+template <>
+struct hash<apache::geode::client::PdxType> {
+ typedef apache::geode::client::PdxType argument_type;
+ typedef size_t result_type;
+ result_type operator()(const argument_type& val) const {
+ return val.hashcode();
Review comment:
Technically we can take the `hashcode` method off the `PdxType` and line
it here. The hash is only used for `std::hash` so let's not split it across the
two classes.
##########
File path: cppcache/src/PdxType.hpp
##########
@@ -205,10 +205,25 @@ class PdxType : public internal::DataSerializableInternal,
// This is for PdxType as key in std map.
bool operator<(const PdxType& other) const;
Review comment:
If we aren't going to use the std::map anymore perhaps we should remove
this `operator<`.
----------------------------------------------------------------
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]
