Copilot commented on code in PR #382:
URL: https://github.com/apache/dubbo-go-hessian2/pull/382#discussion_r2296490719
##########
decode_benchmark_test.go:
##########
@@ -139,3 +164,82 @@ func generateLargeMap(depth int, size int)
map[string]interface{} {
func generateRandomString() string {
return
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"[rand.Int31n(20):]
}
+
+// verifyMapStructure performs structured comparison instead of string
comparison
+// to avoid issues with floating point precision and map iteration order
+func verifyMapStructure(original map[string]interface{}, decoded
map[interface{}]interface{}, t *testing.T) bool {
+ // Check if basic structure elements exist
+ for key, originalValue := range original {
+ decodedValue, exists := decoded[key]
+ if !exists {
+ t.Logf("Key %s missing in decoded map", key)
+ return false
+ }
+
+ // For nested maps, recursively verify
+ if originalMap, ok := originalValue.(map[string]interface{});
ok {
+ if decodedMap, ok :=
decodedValue.(map[interface{}]interface{}); ok {
+ if !verifyMapStructure(originalMap, decodedMap,
t) {
+ return false
+ }
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected map, got %T", key, decodedValue)
+ return false
+ }
+ continue
+ }
+
+ // For slices, verify basic structure
+ if originalSlice, ok := originalValue.([]interface{}); ok {
+ if decodedSlice, ok := decodedValue.([]interface{}); ok
{
+ if len(originalSlice) != len(decodedSlice) {
+ t.Logf("Slice length mismatch for key
%s: expected %d, got %d", key, len(originalSlice), len(decodedSlice))
+ return false
+ }
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected slice, got %T", key, decodedValue)
+ return false
+ }
+ continue
+ }
+
+ // For basic types, we can do direct comparison
+ // but be tolerant of floating point precision differences and
type conversions
+ if originalFloat, ok := originalValue.(float32); ok {
+ // Hessian may convert float32 to float64
+ var decodedFloat float64
+ if f32, ok := decodedValue.(float32); ok {
+ decodedFloat = float64(f32)
+ } else if f64, ok := decodedValue.(float64); ok {
+ decodedFloat = f64
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected float, got %T", key, decodedValue)
+ return false
+ }
+ // Allow small floating point differences
+ if abs64(float64(originalFloat)-decodedFloat) > 1e-6 {
+ t.Logf("Float value mismatch for key %s:
expected %f, got %f", key, originalFloat, decodedFloat)
+ return false
+ }
+ continue
+ }
+ }
+
+ return true
+}
+
+// abs returns the absolute value of a float32
+func abs(x float32) float32 {
+ if x < 0 {
+ return -x
+ }
+ return x
+}
+
+// abs64 returns the absolute value of a float64
+func abs64(x float64) float64 {
+ if x < 0 {
+ return -x
+ }
+ return x
+}
Review Comment:
The function 'abs64' duplicates functionality available in the standard
library. Consider using math.Abs() from the math package instead of
implementing a custom absolute value function.
```suggestion
```
##########
decode_benchmark_test.go:
##########
@@ -139,3 +164,82 @@ func generateLargeMap(depth int, size int)
map[string]interface{} {
func generateRandomString() string {
return
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"[rand.Int31n(20):]
}
+
+// verifyMapStructure performs structured comparison instead of string
comparison
+// to avoid issues with floating point precision and map iteration order
+func verifyMapStructure(original map[string]interface{}, decoded
map[interface{}]interface{}, t *testing.T) bool {
+ // Check if basic structure elements exist
+ for key, originalValue := range original {
+ decodedValue, exists := decoded[key]
+ if !exists {
+ t.Logf("Key %s missing in decoded map", key)
+ return false
+ }
+
+ // For nested maps, recursively verify
+ if originalMap, ok := originalValue.(map[string]interface{});
ok {
+ if decodedMap, ok :=
decodedValue.(map[interface{}]interface{}); ok {
+ if !verifyMapStructure(originalMap, decodedMap,
t) {
+ return false
+ }
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected map, got %T", key, decodedValue)
+ return false
+ }
+ continue
+ }
+
+ // For slices, verify basic structure
+ if originalSlice, ok := originalValue.([]interface{}); ok {
+ if decodedSlice, ok := decodedValue.([]interface{}); ok
{
+ if len(originalSlice) != len(decodedSlice) {
+ t.Logf("Slice length mismatch for key
%s: expected %d, got %d", key, len(originalSlice), len(decodedSlice))
+ return false
+ }
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected slice, got %T", key, decodedValue)
+ return false
+ }
+ continue
+ }
+
+ // For basic types, we can do direct comparison
+ // but be tolerant of floating point precision differences and
type conversions
+ if originalFloat, ok := originalValue.(float32); ok {
+ // Hessian may convert float32 to float64
+ var decodedFloat float64
+ if f32, ok := decodedValue.(float32); ok {
+ decodedFloat = float64(f32)
+ } else if f64, ok := decodedValue.(float64); ok {
+ decodedFloat = f64
+ } else {
+ t.Logf("Value type mismatch for key %s:
expected float, got %T", key, decodedValue)
+ return false
+ }
+ // Allow small floating point differences
+ if abs64(float64(originalFloat)-decodedFloat) > 1e-6 {
+ t.Logf("Float value mismatch for key %s:
expected %f, got %f", key, originalFloat, decodedFloat)
+ return false
+ }
+ continue
+ }
+ }
+
+ return true
+}
+
+// abs returns the absolute value of a float32
+func abs(x float32) float32 {
+ if x < 0 {
+ return -x
+ }
+ return x
+}
+
+// abs64 returns the absolute value of a float64
+func abs64(x float64) float64 {
+ if x < 0 {
+ return -x
+ }
+ return x
+}
Review Comment:
The function 'abs' duplicates functionality available in the standard
library. Consider using math.Abs() from the math package instead of
implementing a custom absolute value function.
```suggestion
// Use math.Abs for absolute value calculations.
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
