pbacsko commented on code in PR #923:
URL: https://github.com/apache/yunikorn-core/pull/923#discussion_r1704141345
##########
pkg/common/resources/resources.go:
##########
@@ -645,6 +645,40 @@ func Equals(left, right *Resource) bool {
return true
}
+// DeepEquals Compare the resources equal returns the specific values for
following cases:
+// left right return
+// nil nil true
+// nil <set> false
+// <set> nil false
+// <set> <set> true/false *based on the individual resource type presence
and its Quantity values
+func DeepEquals(left, right *Resource) bool {
Review Comment:
The corresponding unit test needs to be improved. There should not be
uncovered paths.
##########
pkg/common/resources/resources.go:
##########
@@ -645,6 +645,40 @@ func Equals(left, right *Resource) bool {
return true
}
+// DeepEquals Compare the resources equal returns the specific values for
following cases:
+// left right return
+// nil nil true
+// nil <set> false
+// <set> nil false
+// <set> <set> true/false *based on the individual resource type presence
and its Quantity values
Review Comment:
This comment and potentially the one for `Equals()` need to be enhanced.
It's exactly the same and one has to look at the code to figure out the
differences.
##########
pkg/common/resources/resources.go:
##########
@@ -645,6 +645,40 @@ func Equals(left, right *Resource) bool {
return true
}
+// DeepEquals Compare the resources equal returns the specific values for
following cases:
+// left right return
+// nil nil true
+// nil <set> false
+// <set> nil false
+// <set> <set> true/false *based on the individual resource type presence
and its Quantity values
+func DeepEquals(left, right *Resource) bool {
+ if left == right {
+ return true
+ }
+ if left == nil || right == nil {
+ return false
+ }
+ for k, v := range left.Resources {
+ if val, ok := right.Resources[k]; ok {
+ if val != v {
+ return false
+ }
+ } else {
+ return false
+ }
+ }
+ for k, v := range right.Resources {
+ if val, ok := left.Resources[k]; ok {
+ if val != v {
+ return false
+ }
+ } else {
+ return false
+ }
+ }
Review Comment:
I don't think we need two loops, you can get away with a length check first:
```
if len(right.Resources) != len(left.Resources) {
return false
}
for k, v := range left.Resources {
if val, ok := right.Resources[k]; ok {
if val != v {
return false
}
} else {
return false
}
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
