Skip to content

Commit

Permalink
Reduce code complexity
Browse files Browse the repository at this point in the history
  • Loading branch information
@svyotov
svyotov committed Jan 12, 2020
1 parent 7e445ff commit fce15cb
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 78 deletions.
2 changes: 1 addition & 1 deletion issue66_test.go
View file
Expand Up @@ -21,7 +21,7 @@ func TestPrivateSlice(t *testing.T) {
t.Fatalf("Error during the merge: %v", err)
}
if len(p1.PublicStrings) != 3 {
t.Error("5 elements should be in 'PublicStrings' field")
t.Error("3 elements should be in 'PublicStrings' field, when no append")
}
if len(p1.privateStrings) != 2 {
t.Error("2 elements should be in 'privateStrings' field")
Expand Down
107 changes: 35 additions & 72 deletions merge.go
View file
Expand Up @@ -17,10 +17,8 @@ import (
func hasExportedField(dst reflect.Value) (exported bool) {
for i, n := 0, dst.NumField(); i < n; i++ {
field := dst.Type().Field(i)
if field.Anonymous && dst.Field(i).Kind() == reflect.Struct {
exported = exported || hasExportedField(dst.Field(i))
} else {
exported = exported || isFieldExported(field)
if isFieldExported(field) {
return true
}
}
return
Expand Down Expand Up @@ -68,6 +66,7 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
if !src.IsValid() {
return
}

if dst.CanAddr() {
addr := dst.UnsafeAddr()
h := 17 * addr
Expand All @@ -89,6 +88,11 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
}
}

if dst.IsValid() && src.IsValid() && src.Type() != dst.Type() {
err = fmt.Errorf("cannot append two different types (%s, %s)", src.Kind(), dst.Kind())
return
}

switch dst.Kind() {
case reflect.Struct:
if hasExportedField(dst) {
Expand Down Expand Up @@ -121,11 +125,13 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
} else {
dst = dstCp
}
return
} else {
if (isReflectNil(dst) || overwrite) && (!isEmptyValue(src) || overwriteWithEmptySrc) {
dst = src
}
}

case reflect.Map:
if dst.IsNil() && !src.IsNil() {
if dst.CanSet() {
Expand All @@ -137,91 +143,52 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
}
for _, key := range src.MapKeys() {
srcElement := src.MapIndex(key)
dstElement := dst.MapIndex(key)
if !srcElement.IsValid() {
continue
}
dstElement := dst.MapIndex(key)
if dst.MapIndex(key).IsValid() {
k := dstElement.Interface()
dstElement = reflect.ValueOf(k)
}

switch srcElement.Kind() {
case reflect.Chan, reflect.Func, reflect.Map, reflect.Interface, reflect.Slice:
if srcElement.IsNil() {
continue
}
fallthrough
default:
if !srcElement.CanInterface() {
continue
}
switch reflect.TypeOf(srcElement.Interface()).Kind() {
case reflect.Struct:
fallthrough
case reflect.Ptr:
fallthrough
case reflect.Map:
srcMapElm := srcElement
dstMapElm := dstElement
if srcMapElm.CanInterface() {
srcMapElm = reflect.ValueOf(srcMapElm.Interface())
if dstMapElm.IsValid() {
dstMapElm = reflect.ValueOf(dstMapElm.Interface())
}
}
dstMapElm, err = deepMerge(dstMapElm, srcMapElm, visited, depth+1, config)
if err != nil {
return
}
dst.SetMapIndex(key, dstMapElm)
case reflect.Slice:
srcSlice := reflect.ValueOf(srcElement.Interface())

var dstSlice reflect.Value
if !dstElement.IsValid() || isReflectNil(dstElement) {
dstSlice = reflect.MakeSlice(srcSlice.Type(), 0, srcSlice.Len())
} else {
dstSlice = reflect.ValueOf(dstElement.Interface())
}

if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
if typeCheck && srcSlice.Type() != dstSlice.Type() {
return fmt.Errorf("cannot override two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type())
}
dstSlice = srcSlice
} else if config.AppendSlice {
if srcSlice.Type() != dstSlice.Type() {
return fmt.Errorf("cannot append two slices with different type (%s, %s)", srcSlice.Type(), dstSlice.Type())
}
dstSlice = reflect.AppendSlice(dstSlice, srcSlice)
}
dst.SetMapIndex(key, dstSlice)
if isReflectNil(srcElement) {
if overwrite || isReflectNil(dstElement) {
dst.SetMapIndex(key, srcElement)
}
continue
}
if dstElement.IsValid() && !isEmptyValue(dstElement) && (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Map || reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Slice) || (reflect.TypeOf(srcElement.Interface()).Kind() == reflect.Struct) {
if !srcElement.CanInterface() {
continue
}

if srcElement.IsValid() && ((srcElement.Kind() != reflect.Ptr && overwrite) || !dstElement.IsValid() || isEmptyValue(dstElement)) {
if dst.IsNil() {
dst.Set(reflect.MakeMap(dst.Type()))
if srcElement.CanInterface() {
srcElement = reflect.ValueOf(srcElement.Interface())
if dstElement.IsValid() {
dstElement = reflect.ValueOf(dstElement.Interface())
}
dst.SetMapIndex(key, srcElement)
}
dstElement, err = deepMerge(dstElement, srcElement, visited, depth+1, config)
if err != nil {
return
}
dst.SetMapIndex(key, dstElement)

}
case reflect.Slice:
if !dst.CanSet() {
break
}
if (!isEmptyValue(src) || overwriteWithEmptySrc || overwriteSliceWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
dst.Set(src)
newSlice := dst
if (!isEmptyValue(src) || overwriteWithEmptySrc) && (overwrite || isEmptyValue(dst)) && !config.AppendSlice {
newSlice = src
} else if config.AppendSlice {
if src.Type() != dst.Type() {
err = fmt.Errorf("cannot append two slice with different type (%s, %s)", src.Type(), dst.Type())
return
}
dst.Set(reflect.AppendSlice(dst, src))
newSlice = reflect.AppendSlice(dst, src)
}
if dst.CanSet() {
dst.Set(newSlice)
} else {
dst = newSlice
}
case reflect.Ptr, reflect.Interface:
if isReflectNil(src) {
Expand All @@ -237,7 +204,6 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
dst = src
}
}
<<<<<<< HEAD
}
break
}
Expand All @@ -247,9 +213,6 @@ func deepMerge(dstIn, src reflect.Value, visited map[uintptr]*visit, depth int,
if dst.CanSet() && (overwrite || isEmptyValue(dst)) {
dst.Set(src)
}
=======

>>>>>>> semi working
} else if src.Kind() == reflect.Ptr {
if dst, err = deepMerge(dst.Elem(), src.Elem(), visited, depth+1, config); err != nil {
return
Expand Down
23 changes: 18 additions & 5 deletions mergo_test.go
View file
Expand Up @@ -345,13 +345,13 @@ func TestEmptyToNotEmptyMaps(t *testing.T) {
}

func TestMapsWithOverwrite(t *testing.T) {
m := map[string]simpleTest{
dst := map[string]simpleTest{
"a": {}, // overwritten by 16
"b": {42}, // overwritten by 0, as map Value is not addressable and it doesn't check for b is set or not set in `n`
"c": {13}, // overwritten by 12
"d": {61},
}
n := map[string]simpleTest{
src := map[string]simpleTest{
"a": {16},
"b": {},
"c": {12},
Expand All @@ -365,12 +365,12 @@ func TestMapsWithOverwrite(t *testing.T) {
"e": {14},
}

if err := MergeWithOverwrite(&m, n); err != nil {
if err := MergeWithOverwrite(&dst, src); err != nil {
t.Fatalf(err.Error())
}

if !reflect.DeepEqual(m, expect) {
t.Fatalf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", m, expect)
if !reflect.DeepEqual(dst, expect) {
t.Fatalf("Test failed:\ngot :\n%#v\n\nwant :\n%#v\n\n", dst, expect)
}
}

Expand Down Expand Up @@ -885,6 +885,7 @@ func TestBooleanPointer(t *testing.T) {
}

func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) {
<<<<<<< HEAD
testCases := []struct {
name string
options []func(*Config)
Expand All @@ -900,6 +901,13 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) {
[]func(*Config){WithOverride, WithTypeCheck},
"cannot override two slices with different type",
},
=======
dst := map[string]interface{}{
"foo": []string{"a", "b"},
}
src := map[string]interface{}{
"foo": []int{1, 2},
>>>>>>> Reduce code complexity
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -910,10 +918,15 @@ func TestMergeMapWithInnerSliceOfDifferentType(t *testing.T) {
"foo": []int{1, 2},
}

<<<<<<< HEAD
if err := Merge(&src, &dst, tc.options...); err == nil || !strings.Contains(err.Error(), tc.err) {
t.Fatalf("expected %q, got %q", tc.err, err)
}
})
=======
if err := Merge(&dst, src, WithOverride, WithAppendSlice); err == nil {
t.Fatal("expected an error, got nothing")
>>>>>>> Reduce code complexity
}
}

Expand Down

0 comments on commit fce15cb

Please sign in to comment.