Reflect Struct Comparison
5,772 views
Skip to first unread message
Luke Mauldin
Mar 30, 2012, 7:59:52 AM3/30/12
to golan...@googlegroups.com
I have the the task of comparing two structs (of the same type) for equality and returning the field level differences. One of the restrictions is that I only have to support the types int, bool, float, string, and time.Time. I have built the following reflection routine and it works correctly for int, float, bool and string but I cannot figure out how do the comparison for time.Time because I am getting the compiler error "type time.Time is not an expression". Does anyone have any suggestions? Also, looking at the overall code, is there a better or more efficient way I can solve this problem?
type CompareResult struct {
FieldName string
Value1 string
Value2 string
}
func Compare (struct1 interface{}, struct2 interface{}) ([]*CompareResult, error) {
if struct1 == nil || struct2 == nil {
return nil, errors.New("One of the inputs cannot be nil.")
}
structVal1 := reflect.ValueOf(struct1)
structVal2 := reflect.ValueOf(struct2)
structType := reflect.TypeOf(struct1)
if !(structVal1.Kind() == reflect.Struct && structVal2.Kind() == reflect.Struct) {
return nil, errors.New("Types must both be structs.")
}
if structVal1.Type() != structVal2.Type() {
return nil, errors.New("Structs must be the same type.")
}
//if structVal1.IsNil() || structVal2.IsNil() {
// return nil, errors.New("Structs cannot be nil.")
//}
numFields := structVal1.NumField()
results := make([]*CompareResult, 0, numFields)
for i := 0; i < numFields; i++ {
//Get values of the structure's fields
val1 := structVal1.Field(i)
val2 := structVal2.Field(i)
//If the values are pointers, get the value of the pointers
isPtr := val1.Kind() == reflect.Ptr
isVal1Nil := false
isVal2Nil := false
if isPtr {
isVal1Nil = val1.IsNil()
isVal2Nil = val2.IsNil()
val1 = val1.Elem()
val2 = val2.Elem()
}
//If the type is a pointer and both values are nil, continue the loop
if isPtr && isVal1Nil && isVal2Nil {
continue;
}
switch val1.Kind() {
case reflect.Int:
int1 := val1.Int()
int2 := val2.Int()
if int1 != int2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatInt(int1, 10), strconv.FormatInt(int2, 10) }
results = append(results, result)
}
break;
case reflect.Bool:
bool1 := val1.Bool()
bool2 := val2.Bool()
if bool1 != bool2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatBool(bool1), strconv.FormatBool(bool2) }
results = append(results, result)
}
break;
case reflect.Float32, reflect.Float64:
float1 := val1.Float()
float2 := val2.Float()
if float1 != float2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatFloat(float1, 'f', 2, 64), strconv.FormatFloat(float2, 'f', 2, 64) }
results = append(results, result)
}
case reflect.Struct:
//Handle time.Time
timeType := reflect.TypeOf(time.Time) //This gives me a "type time.Time is not an expression"
if val1.Type() == timeType {
}
break;
case reflect.String:
str1 := val1.String()
str2 := val2.String()
if str1 != str2 {
result := &CompareResult{structType.Field(i).Name, str1, str2 }
results = append(results, result)
}
break;
default:
return nil, errors.New(fmt.Sprintf("Unsupported kind: %v", val1.Kind()))
}
}
return results, nil
}
Rob 'Commander' Pike
Mar 30, 2012, 8:22:35 AM3/30/12
to Luke Mauldin, golan...@googlegroups.com
It seems like the information is all there in the message. Read what
it says: time.Time is not an expression. It's a type. You need an
expression of type time.TIme, and then reflect.TypeOf it. By the way,
do it outside the function; no need to build the reflect.Type every
call.
it says: time.Time is not an expression. It's a type. You need an
expression of type time.TIme, and then reflect.TypeOf it. By the way,
do it outside the function; no need to build the reflect.Type every
call.
Try
var timeType = reflect.TypeOf(time.Now())
or
var timeType = reflect.TypeOf(time.Time{})
But then, since you have a fixed set of types for the fields, you can
avoid reflection inside and just a type switch on the field:
switch v := val1.Interface().(type) {
case int:
// v has type int here.
// etc.
case time.Time:
// etc.
}
-rob
Luke Mauldin
Mar 30, 2012, 9:01:13 AM3/30/12
to golan...@googlegroups.com, Luke Mauldin
If I make the chance to use val1.Interface().(type) how then do I handle pointers to string, int, etc..?
Luke Mauldin
Mar 30, 2012, 9:05:46 AM3/30/12
to golan...@googlegroups.com, Luke Mauldin
I think I may have answered my own question. It seems possible to just do a check for *int like the following:
case *int:
val2 := field2.Interface().(*int)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, strconv.Itoa(*val1), strconv.Itoa(*val2) }
results = append(results, result)
}
break;
Is that the best way to do it?
Rob 'Commander' Pike
Mar 30, 2012, 9:08:08 AM3/30/12
to Luke Mauldin, golan...@googlegroups.com
You can do it that way (although I believe you want to compare what
the integer points to, not the pointer itself). You can also make it a
hybrid, flattening the pointers by reflection (use reflect.Indirect
though), then type switch on the value types.
the integer points to, not the pointer itself). You can also make it a
hybrid, flattening the pointers by reflection (use reflect.Indirect
though), then type switch on the value types.
-rob
Luke Mauldin
Mar 30, 2012, 9:45:27 AM3/30/12
to golan...@googlegroups.com, Luke Mauldin
Rob,
Thanks for all of your feedback. I have posted my final solution before for anyone else who may have the same problem or is interested. Please let me know if there is anything else I can do to improve it.
import (
"reflect"
"errors"
"fmt"
"strconv"
"time"
"util/config"
)
type CompareResult struct {
FieldName string
Value1 string
Value2 string
}
func Compare (struct1 interface{}, struct2 interface{}) ([]*CompareResult, error) {
if struct1 == nil || struct2 == nil {
return nil, errors.New("One of the inputs cannot be nil.")
}
structVal1 := reflect.ValueOf(struct1)
structVal2 := reflect.ValueOf(struct2)
structType := reflect.TypeOf(struct1)
if !(structVal1.Kind() == reflect.Struct && structVal2.Kind() == reflect.Struct) {
return nil, errors.New("Types must both be structs.")
}
if structVal1.Type() != structVal2.Type() {
return nil, errors.New("Structs must be the same type.")
}
numFields := structVal1.NumField()
results := make([]*CompareResult, 0, numFields)
for i := 0; i < numFields; i++ {
//Get values of the structure's fields
field1 := structVal1.Field(i)
field2 := structVal2.Field(i)
//If the field name is unexported, skip
if structType.Field(i). PkgPath != "" {
continue
}
//Handle nil pointers
isPtr := field1.Kind() == reflect.Ptr
isField1Nil := false
isField2Nil := false
if isPtr {
isField1Nil = field1.IsNil()
isField2Nil = field2.IsNil()
}
//If both fields are nil, continue the loop
if isPtr && isField1Nil && isField2Nil {
continue;
}
switch val1 := field1.Interface().(type) {
case int:
val2 := field2.Interface().(int)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, strconv.Itoa(val1), strconv.Itoa(val2) }
results = append(results, result)
}
break;
case *int:
val2 := field2.Interface().(*int)
var int1 int
var int2 int
if isField1Nil {
int1 = 0
} else {
int1 = *val1
}
if isField2Nil {
int2 = 0
} else {
int2 = *val2
}
if int1 != int2 {
result := &CompareResult{structType.Field(i).Name, strconv.Itoa(int1), strconv.Itoa(int2) }
results = append(results, result)
}
break;
case bool:
val2 := field2.Interface().(bool)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatBool(val1), strconv.FormatBool(val2) }
results = append(results, result)
}
break;
case *bool:
val2 := field2.Interface().(*bool)
var bool1 bool
var bool2 bool
if isField1Nil {
bool1 = false
} else {
bool1 = *val1
}
if isField2Nil {
bool2 = false
} else {
bool2 = *val2
}
if bool1 != bool2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatBool(bool1), strconv.FormatBool(bool2) }
results = append(results, result)
}
break;
case float64:
val2 := field2.Interface().(float64)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatFloat(val1, 'f', 2, 64), strconv.FormatFloat(val2, 'f', 2, 64) }
results = append(results, result)
}
break;
case *float64:
val2 := field2.Interface().(*float64)
var float1 float64
var float2 float64
if isField1Nil {
float1 = 0
} else {
float1 = *val1
}
if isField2Nil {
float2 = 0
} else {
float2 = *val2
}
if float1 != float2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatFloat(float1, 'f', 2, 64), strconv.FormatFloat(float2, 'f', 2, 64) }
results = append(results, result)
}
break;
case string:
val2 := field2.Interface().(string)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, val1, val2 }
results = append(results, result)
}
break;
case *string:
val2 := field2.Interface().(*string)
var string1 string
var string2 string
if isField1Nil {
string1 = ""
} else {
string1 = *val1
}
if isField2Nil {
string2 = ""
} else {
string2 = *val2
}
if string1 != string2 {
result := &CompareResult{structType.Field(i).Name, string1, string2 }
results = append(results, result)
}
break;
case time.Time:
val2 := field2.Interface().(time.Time)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, val1.Format(config.DateTimeFormat), val2.Format(config.DateTimeFormat) }
results = append(results, result)
}
break;
case *time.Time:
val2 := field2.Interface().(*time.Time)
var time1 string
var time2 string
if isField1Nil {
time1 = ""
} else {
time1 = val1.Format(config.DateTimeFormat)
}
if isField2Nil {
time2 = ""
} else {
time2 = val2.Format(config.DateTimeFormat)
}
if time1 != time2 {
result := &CompareResult{structType.Field(i).Name, time1, time2 }
results = append(results, result)
}
break;
default:
return nil, errors.New(fmt.Sprintf("Unsupported type: %v", val1))
}
}
return results, nil
Kyle Lemons
Mar 30, 2012, 1:17:31 PM3/30/12
to Luke Mauldin, golan...@googlegroups.com
On Fri, Mar 30, 2012 at 6:45 AM, Luke Mauldin <lukem...@gmail.com> wrote:
Rob,Thanks for all of your feedback. I have posted my final solution before for anyone else who may have the same problem or is interested. Please let me know if there is anything else I can do to improve it.import ("reflect""errors""fmt""strconv""time""util/config")type CompareResult struct {FieldName stringValue1 stringValue2 string
I'd go with Value1, Value2 interface{} so you can store them directly; you may also want to make CompareResult a list of (structures containing) fields and their values and a String() method for easy printing. I'd also only include differences in there.
}func Compare (struct1 interface{}, struct2 interface{}) ([]*CompareResult, error) {
You may want to return something like (equal bool, differences []CompareResult, err error) which makes trivial uses simpler.
}
These names are somewhat verbose for variables that will be used frequently. v1 and v2 and st are probably explicit enough.
I would precede this check with a loop that flattens pointers. Something like
for v1.Kind() == reflect.Pointer {
if v1.IsNil() { return blah }
v1 = v1.Elem()
}
and the same for v2. That way you can pass one or both structs as pointers to struct values.
return nil, errors.New("Types must both be structs.")}
I would use fmt.Errorf and include the types of the structs; it'll help anyone using the function.
}
If you take the suggestion to only include differences, I wouldn't set capacity and let append() do its thing.
also, f1 and f2 are probably explicit enough. This is just style/bikeshedding though.
gofmt would take out that space; it would also realign the cases.
continue}
If you're not nesting structures, you can probably replace the rest of this with something like
switch k := f1.Kind(); k {
case reflect.Map, reflect.Chan, reflect.Slice, reflect.Interface:
// return error cannot compare (though you can compare slices manually if you wish and some interfaces may contain comparable values)
case reflect.Ptr:
if f1.IsNil() != f2.IsNil() {
// add to inequal list
continue
}
f1, f2 = f1.Elem(), f2.Elem()
fallthrough
default:
if f1.Interface() != f2.Interface() {
// add to inequal list
Steven Blenkinsop
Mar 30, 2012, 1:30:57 PM3/30/12
to Luke Mauldin, golan...@googlegroups.com, Luke Mauldin
Rob,Thanks for all of your feedback. I have posted my final solution before for anyone else who may have the same problem or is interested. Please let me know if there is anything else I can do to improve it.
if struct1 == nil || struct2 == nil {
return nil, errors.New("One of the inputs cannot be nil.")}
I wouldn't handle this case, at least not as an error return. The only way to get nil interfaces is programmer error. That's not something that has to be handled at runtime, it's a bug in the code that has to be fixed by the developer. Of course, it's your choice, but the tendency I see is to crash on misuse.
//Handle nil pointersisPtr := field1.Kind() == reflect.PtrisField1Nil := falseisField2Nil := falseif isPtr {isField1Nil = field1.IsNil()isField2Nil = field2.IsNil()}}
You're doing too much work here. Remember that && short circuits. If isPtr is false, the rest of the condition won't be evaluated. So you can just put the IsNil checks directly in the if condition.
I'd use reflect.Indirect to flatten pointers. You would then check IsValid instead of IsNil. You can use reflect.Zero to replace an invalid value with the zero value for the other value's type.
switch val1 := field1.Interface().(type) {
There are only really four cases here, especially if you used reflect.Indirect above:
case int, bool, string:
// use fmt.Sprint to get the string representation of Interface results
case float64:
// use strconv.FormatFloat, but only because you want to specify a precision
// otherwise it can be lumped in with the previous case
case time.Time
// this is so you can specify the format you want
// if you wanted to use the default format, you could lump this in with the first case
default:
// handle unrecognized types
break;
Neither the semicolon nor the break statement is needed here. Go switches don't implicitly fall through. There is a fallthrough statement to explicitly fall through, but it doesn't work in type switches.
Luke Mauldin
Mar 30, 2012, 2:39:41 PM3/30/12
to golan...@googlegroups.com
Kyle & Steven,
Thank you both very much for your feedback. I was able to improve my implementation and I have it posted below in case anyone would like to use it for reference.
package compare
import (
"errors"
"fmt"
"reflect"
"strconv"
"time"
"util/config"
)
type CompareResult struct {
FieldName string
Value1 string
Value2 string
}
func Compare(struct1 interface{}, struct2 interface{}) (areEqual bool, differences []*CompareResult, err error) {
if struct1 == nil || struct2 == nil {
return false, nil, errors.New("One of the inputs cannot be nil.")
}
structVal1 := reflect.ValueOf(struct1)
structVal2 := reflect.ValueOf(struct2)
//Handle pointers
structVal1 = reflect.Indirect(structVal1)
structVal2 = reflect.Indirect(structVal2)
if !structVal1.IsValid() || !structVal2.IsValid() {
return false, nil, errors.New(fmt.Sprintf("Types cannot be nil. structVal1 %v - structVal2 %v", structVal1.IsValid(), structVal2.IsValid()))
}
//Cache struct type
structType := structVal1.Type()
if !(structVal1.Kind() == reflect.Struct && structVal2.Kind() == reflect.Struct) {
return false, nil, errors.New(fmt.Sprintf("Types must both be structs. Kind1: %v, Kind2 :v", structVal1.Kind(), structVal2.Kind()))
}
if structVal1.Type() != structVal2.Type() {
return false, nil, errors.New(fmt.Sprintf("Structs must be the same type. Struct1 %v - Stuct2 -%v", structVal1.Type(), structVal2.Type()))
}
numFields := structVal1.NumField()
differences = make([]*CompareResult, 0)
for i := 0; i < numFields; i++ {
//Get values of the structure's fields
field1 := structVal1.Field(i)
field2 := structVal2.Field(i)
//If the field name is unexported, skip
if structType.Field(i).PkgPath != "" {
continue
}
//Handle nil pointers
field1 = reflect.Indirect(field1)
field2 = reflect.Indirect(field2)
field1IsValid := field1.IsValid()
field2IsValid := field2.IsValid()
//If both fields are invalid, continue the loop
if !field1IsValid && !field2IsValid {
continue
}
//If only one field is not valid, replace it with the valid type of the other field
if !field1IsValid {
field1 = reflect.Zero(field2.Type())
}
if !field2IsValid {
field2 = reflect.Zero(field1.Type())
}
switch val1 := field1.Interface().(type) {
case int, bool, string:
val2 := field2.Interface()
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, fmt.Sprint(val1), fmt.Sprint(val2)}
differences = append(differences, result)
}
case float64:
val2 := field2.Interface().(float64)
if val1 != val2 {
result := &CompareResult{structType.Field(i).Name, strconv.FormatFloat(val1, 'f', 2, 64), strconv.FormatFloat(val2, 'f', 2, 64)}
differences = append(differences, result)
}
case time.Time:
val2 := field2.Interface().(time.Time)
if val1 != val2 {
time1 := val1.Format(config.DisplayDateTimeFormat)
time2 := val2.Format(config.DisplayDateTimeFormat)
result := &CompareResult{structType.Field(i).Name, time1, time2}
differences = append(differences, result)
}
default:
return false, nil, errors.New(fmt.Sprintf("Unsupported type: %v", val1))
}
}
areEqual = len(differences) == 0
return areEqual, differences, nil
Kyle Lemons
Mar 30, 2012, 3:09:25 PM3/30/12
to Luke Mauldin, golan...@googlegroups.com
On Fri, Mar 30, 2012 at 11:39 AM, Luke Mauldin <lukem...@gmail.com> wrote:
Kyle & Steven,Thank you both very much for your feedback. I was able to improve my implementation and I have it posted below in case anyone would like to use it for reference.
You could make it a publicly accessible package on github or something.
Also, did you unit test this?
package compareimport ("errors""fmt""reflect""strconv""time""util/config")type CompareResult struct {FieldName stringValue1 stringValue2 string
I still think these should be interface{}
}func Compare(struct1 interface{}, struct2 interface{}) (areEqual bool, differences []*CompareResult, err error) {if struct1 == nil || struct2 == nil {return false, nil, errors.New("One of the inputs cannot be nil.")}
You do paired things like this frequently, and you can do them with compound assignment if you want.
Do you want to hande non-pointers too?
structVal1 = reflect.Indirect(structVal1)structVal2 = reflect.Indirect(structVal2)if !structVal1.IsValid() || !structVal2.IsValid() {return false, nil, errors.New(fmt.Sprintf("Types cannot be nil. structVal1 %v - structVal2 %v", structVal1.IsValid(), structVal2.IsValid()))}
DeMorgan says v1.Kind != struct || v2.Kind != struct :)
return false, nil, errors.New(fmt.Sprintf("Types must both be structs. Kind1: %v, Kind2 :v", structVal1.Kind(), structVal2.Kind()))}if structVal1.Type() != structVal2.Type() {return false, nil, errors.New(fmt.Sprintf("Structs must be the same type. Struct1 %v - Stuct2 -%v", structVal1.Type(), structVal2.Type()))}numFields := structVal1.NumField()differences = make([]*CompareResult, 0)
Redundant (it's already a nil slice which works fine with append)
for i := 0; i < numFields; i++ {//Get values of the structure's fieldsfield1 := structVal1.Field(i)field2 := structVal2.Field(i)//If the field name is unexported, skipif structType.Field(i).PkgPath != "" {continue}
What about non-pointers?
field1 = reflect.Indirect(field1)field2 = reflect.Indirect(field2)
again, parallel assignment++
field1IsValid := field1.IsValid()field2IsValid := field2.IsValid()
These can be in an initializer for a switch
switch valid1, valid2 := spam, eggs; {
case valid1 && valid2:
case valid1:
// 2 invalid
continue
case valid2:
// 1 invalid
continue
default:
// both invalid
continue
}
}}if !field2IsValid {field2 = reflect.Zero(field1.Type())}
switch val1, val2 := field1.Interface(), field2.Interface(); val1.(type) {
case int, bool, string:val2 := field2.Interface()if val1 != val2 {result := &CompareResult{structType.Field(i).Name, fmt.Sprint(val1), fmt.Sprint(val2)}differences = append(differences, result)}case float64:val2 := field2.Interface().(float64)if val1 != val2 {
I think you want an epsilon check here. Otherwise it rolls into the above case.
result := &CompareResult{structType.Field(i).Name, strconv.FormatFloat(val1, 'f', 2, 64), strconv.FormatFloat(val2, 'f', 2, 64)}differences = append(differences, result)}case time.Time:
This should roll into the first case, unless you want to use t1.Equal(t2) to ignore timezone.
val2 := field2.Interface().(time.Time)if val1 != val2 {time1 := val1.Format(config.DisplayDateTimeFormat)time2 := val2.Format(config.DisplayDateTimeFormat)
If they're interface{} you can use them directly.
result := &CompareResult{structType.Field(i).Name, time1, time2}differences = append(differences, result)}default:return false, nil, errors.New(fmt.Sprintf("Unsupported type: %v", val1))
fmt.Errorf (and all above)
}}
you can inline this in the return
Steven Blenkinsop
Mar 30, 2012, 4:08:32 PM3/30/12
to Kyle Lemons, Luke Mauldin, golan...@googlegroups.com
On 2012-03-30, at 3:09 PM, Kyle Lemons <kev...@google.com> wrote:
//Handle nil pointersWhat about non-pointers?
From the documentation:
"If v is not a pointer, Indirect returns v."
The fields should always be valid, so all cases are handled.
Kyle Lemons
Mar 30, 2012, 5:20:23 PM3/30/12
to Steven Blenkinsop, Luke Mauldin, golan...@googlegroups.com
Oh! Would you look at that. I remember reading the doc and making a mental map that "*x == Indirect() == val.Elem()" and completely missed that part.
Handy.
Steven Blenkinsop
Mar 30, 2012, 5:57:15 PM3/30/12
to Kyle Lemons, Luke Mauldin, golan...@googlegroups.com
However, because you mentioned it, there *is* a case that isn't handled correctly.
Luke, if a field has an interface type, the function will handle it if it contains a value but not a pointer. Assuming you don't want to handle this case, you should add a check for Kind() == reflect.Interface before the type switch, and return an error in that case. If you do want to handle it, then you have to consider how many levels of indirection you want to support.
Luke Mauldin
Mar 30, 2012, 6:16:41 PM3/30/12
to golan...@googlegroups.com, Kyle Lemons, Luke Mauldin
Steven,
I think my answer will be that I do not want to handle that case because I have unit tests setup for all of the scenarios that I expect and they are all passing. However, I do not fully understanding the scenario you are describing. Can you please post a small code example that would demonstrate the case you are describing?
Luke
Luke Mauldin
Mar 30, 2012, 6:46:30 PM3/30/12
to golan...@googlegroups.com, Kyle Lemons, Luke Mauldin
I have taken everyone's suggestions into account everyone's suggestions and updated my code and the unit tests. I am posting them both for review and in case anyone else has a similar problem. I am relatively new to the Go programming language and I have learned a tremendous amount from everyone's suggestions. Please let me know if you still see I am missing anything. Thank you so much for your input.
Luke
Steven Blenkinsop
Mar 30, 2012, 10:29:09 PM3/30/12
to Luke Mauldin, golan...@googlegroups.com, Kyle Lemons
On Fri, Mar 30, 2012 at 6:16 PM, Luke Mauldin <lukem...@gmail.com> wrote:
Steven,I think my answer will be that I do not want to handle that case because I have unit tests setup for all of the scenarios that I expect and they are all passing. However, I do not fully understanding the scenario you are describing. Can you please post a small code example that would demonstrate the case you are describing?Luke
Here are test cases that show the problem:
func TestCompareInterfaceWithPtr(t *testing.T) {
type testStruct struct {
I interface{}
}
struct1 := testStruct{new(int)}
struct2 := testStruct{new(int)}
_, _, err := Compare(struct1, struct2)
if err == nil {
t.Errorf("Failed to reject interface field containing a pointer.\n")
}
}
func TestCompareInterfaceWithValue(t *testing.T) {
type testStruct struct {
I interface{}
}
struct1 := testStruct{42}
struct2 := testStruct{43}
_, _, err := Compare(struct1, struct2)
if err == nil {
t.Errorf("Failed to reject interface field containing a value.\n")
}
}
If you remove the check for KInd() == reflect.Interface, TestCompareInterfaceWithValue will fail. This is because when you call Interface(), you can't tell whether the reflect.Value represented the interface or if it represented the value in the interface, which is what the type switch assumes. So, if the type of the value in the interface field is one of the types listed in the type switch, it won't get rejected. TestCompareInterfaceWithPtr succeeds because indirect doesn't reach through the interface to indirect the pointer inside it, so when you call Interface, you get an interface representing a pointer, which doesn't get matched in your type switch. Here's some illustrative code: http://play.golang.org/p/IBfCHoC38q
Steven Blenkinsop
Mar 30, 2012, 11:39:05 PM3/30/12
to Luke Mauldin, golan...@googlegroups.com, Kyle Lemons
I have taken everyone's suggestions into account everyone's suggestions and updated my code and the unit tests. I am posting them both for review and in case anyone else has a similar problem. I am relatively new to the Go programming language and I have learned a tremendous amount from everyone's suggestions. Please let me know if you still see I am missing anything. Thank you so much for your input.Luke
//Initialize differences to ensure length of 0 on return
differences = make([]*CompareResult, 0)
You don't need to do this. From the spec:
The length and capacity of a nil slice, map, or channel are 0.
Prior to the assignment, differences is nil.
switch valid1, valid2 := field1.IsValid(), field2.IsValid(); {
//If both are valid, do nothing
case valid1 && valid2:
//If only field1 is valid, set field2 to reflect.Zero
case valid1:
field2 = reflect.Zero(field1.Type())
//If only field1 is valid, set field2 to reflect.Zero
case valid2:
field1 = reflect.Zero(field2.Type())
//Both are invalid so skip loop body
default:
continue
}
This is fine, but I'd personally invert the logic (check for !valid1 && !valid2, !valid1, !valid2, and remove the default), so that what I'm handling is explicit. It really works either way, it's just a matter of what makes more sense to you.
Other than that, have you noticed that your code no longer really depends on the types of the fields, other than that they are comparable and not interfaces? ;)
Luke Mauldin
Mar 31, 2012, 12:50:12 PM3/31/12
to golan...@googlegroups.com, Luke Mauldin, Kyle Lemons
Steven,
Thank you very much for the example, I understand what you are talking about now. This will not be a scenario that I expect so I think I will just leave in the check for Kind == Interface for now but I appreciate you bringing it to my attention.
Luke
On Friday, March 30, 2012 9:29:09 PM UTC-5, Steven Blenkinsop wrote:
On Friday, March 30, 2012 9:29:09 PM UTC-5, Steven Blenkinsop wrote:
Luke Mauldin
Mar 31, 2012, 12:52:15 PM3/31/12
to golan...@googlegroups.com, Luke Mauldin, Kyle Lemons
Yes, I really like that the code is no longer dependent on the types of the fields, it is more generic now and looks far more like "good Go code" thanks to everyone's input. I also benchmarked it and it runs in about half the time of before.
Luke
On Friday, March 30, 2012 10:39:05 PM UTC-5, Steven Blenkinsop wrote:
iadvi...@gmail.com
Nov 21, 2016, 11:08:00 PM11/21/16
to golang-nuts
can someone attach the improved and efficient code?
Dave Cheney
Nov 22, 2016, 5:09:43 AM11/22/16
to golang-nuts
Please do not reopen to a four year old thread. Instead please start a new thread describing the problem you have, what you tried, and what happened when you tried.
Thanks
iAdvice Edge
Nov 22, 2016, 5:15:30 AM11/22/16
to Dave Cheney, golang-nuts
Hi Dave,
I need to create a common utility which checks whether the struct is empty or not else i can throw error.}
i need to check whether the object coming from rest api i hosted has empty value or it is filled with something. and i want to make the utility not limited to Employee but to use the same for other structs as well.
how can i do it?
Regards,
Parveen Kumar
On Tue, Nov 22, 2016 at 3:39 PM, Dave Cheney <da...@cheney.net> wrote:
Please do not reopen to a four year old thread. Instead please start a new thread describing the problem you have, what you tried, and what happened when you tried.
Thanks
--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/neGz_Rxtxxw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Dave Cheney
Nov 22, 2016, 5:49:26 AM11/22/16
to golang-nuts
If you want to write a general function you will need to use the reflect package, specifically reflect.Value.IsValid to check each field to see if it contains the zero value or not.
iAdvice Edge
Nov 22, 2016, 5:58:31 AM11/22/16
to Dave Cheney, golang-nuts
i am new to Golang.
can you just share some piece of code just to check each field in struct for empty or nil or zero value.On Tue, Nov 22, 2016 at 4:19 PM, Dave Cheney <da...@cheney.net> wrote:
If you want to write a general function you will need to use the reflect package, specifically reflect.Value.IsValid to check each field to see if it contains the zero value or not.
Dave Cheney
Nov 22, 2016, 6:17:34 AM11/22/16
to golang-nuts, da...@cheney.net, iadvi...@gmail.com
This should get you started. https://play.golang.org/p/fKGkJPMToy
On Tuesday, 22 November 2016 21:58:31 UTC+11, iAdvice Edge wrote:
i am new to Golang.can you just share some piece of code just to check each field in struct for empty or nil or zero value.
On Tue, Nov 22, 2016 at 4:19 PM, Dave Cheney <da...@cheney.net> wrote:
If you want to write a general function you will need to use the reflect package, specifically reflect.Value.IsValid to check each field to see if it contains the zero value or not.
--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/neGz_Rxtxxw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages