From f09be95a4367bec53d6e654fbbb43856afe0b27f Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 29 May 2017 12:26:39 +0100 Subject: [PATCH 1/3] use mapEqual where possible --- report/counters.go | 20 +------------------- report/edge_metadatas.go | 17 +---------------- report/node_set.go | 24 ++---------------------- report/sets.go | 17 +---------------- 4 files changed, 5 insertions(+), 73 deletions(-) diff --git a/report/counters.go b/report/counters.go index e5b7bcf3fa..a110330dd7 100644 --- a/report/counters.go +++ b/report/counters.go @@ -115,25 +115,7 @@ func (c Counters) String() string { // DeepEqual tests equality with other Counters func (c Counters) DeepEqual(d Counters) bool { - if (c.psMap == nil) != (d.psMap == nil) { - return false - } else if c.psMap == nil && d.psMap == nil { - return true - } - - if c.psMap.Size() != d.psMap.Size() { - return false - } - - equal := true - c.psMap.ForEach(func(k string, val interface{}) { - if otherValue, ok := d.psMap.Lookup(k); !ok { - equal = false - } else { - equal = equal && reflect.DeepEqual(val, otherValue) - } - }) - return equal + return mapEqual(c.psMap, d.psMap, reflect.DeepEqual) } func (c Counters) fromIntermediate(in map[string]int) Counters { diff --git a/report/edge_metadatas.go b/report/edge_metadatas.go index 2927b67084..4d34e95a97 100644 --- a/report/edge_metadatas.go +++ b/report/edge_metadatas.go @@ -129,22 +129,7 @@ func (c EdgeMetadatas) String() string { // DeepEqual tests equality with other Counters func (c EdgeMetadatas) DeepEqual(d EdgeMetadatas) bool { - if c.Size() != d.Size() { - return false - } - if c.Size() == 0 { - return true - } - - equal := true - c.psMap.ForEach(func(k string, val interface{}) { - if otherValue, ok := d.psMap.Lookup(k); !ok { - equal = false - } else { - equal = equal && reflect.DeepEqual(val, otherValue) - } - }) - return equal + return mapEqual(c.psMap, d.psMap, reflect.DeepEqual) } // CodecEncodeSelf implements codec.Selfer diff --git a/report/node_set.go b/report/node_set.go index 8c73c20997..b302488a29 100644 --- a/report/node_set.go +++ b/report/node_set.go @@ -142,28 +142,8 @@ func (n NodeSet) String() string { } // DeepEqual tests equality with other NodeSets -func (n NodeSet) DeepEqual(i interface{}) bool { - d, ok := i.(NodeSet) - if !ok { - return false - } - - if n.Size() != d.Size() { - return false - } - if n.Size() == 0 { - return true - } - - equal := true - n.psMap.ForEach(func(k string, val interface{}) { - if otherValue, ok := d.psMap.Lookup(k); !ok { - equal = false - } else { - equal = equal && reflect.DeepEqual(val, otherValue) - } - }) - return equal +func (n NodeSet) DeepEqual(o NodeSet) bool { + return mapEqual(n.psMap, o.psMap, reflect.DeepEqual) } func (n NodeSet) toIntermediate() []Node { diff --git a/report/sets.go b/report/sets.go index be1c78b7c8..183ada1deb 100644 --- a/report/sets.go +++ b/report/sets.go @@ -129,22 +129,7 @@ func (s Sets) String() string { // DeepEqual tests equality with other Sets func (s Sets) DeepEqual(t Sets) bool { - if s.Size() != t.Size() { - return false - } - if s.Size() == 0 { - return true - } - - equal := true - s.psMap.ForEach(func(k string, val interface{}) { - if otherValue, ok := t.psMap.Lookup(k); !ok { - equal = false - } else { - equal = equal && reflect.DeepEqual(val, otherValue) - } - }) - return equal + return mapEqual(s.psMap, t.psMap, reflect.DeepEqual) } // CodecEncodeSelf implements codec.Selfer From 87d91c55d913684c3430267a5f3658230a3dc616 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 29 May 2017 12:33:46 +0100 Subject: [PATCH 2/3] use mapToString where possible --- report/edge_metadatas.go | 19 +------------------ report/sets.go | 20 +------------------- 2 files changed, 2 insertions(+), 37 deletions(-) diff --git a/report/edge_metadatas.go b/report/edge_metadatas.go index 4d34e95a97..24b2064012 100644 --- a/report/edge_metadatas.go +++ b/report/edge_metadatas.go @@ -1,10 +1,8 @@ package report import ( - "bytes" "fmt" "reflect" - "sort" "strconv" "github.com/ugorji/go/codec" @@ -109,22 +107,7 @@ func (c EdgeMetadatas) ForEach(fn func(k string, v EdgeMetadata)) { } func (c EdgeMetadatas) String() string { - keys := []string{} - if c.psMap == nil { - c = EmptyEdgeMetadatas - } - for _, k := range c.psMap.Keys() { - keys = append(keys, k) - } - sort.Strings(keys) - - buf := bytes.NewBufferString("{") - for _, key := range keys { - val, _ := c.psMap.Lookup(key) - fmt.Fprintf(buf, "%s: %v, ", key, val) - } - fmt.Fprintf(buf, "}") - return buf.String() + return mapToString(c.psMap) } // DeepEqual tests equality with other Counters diff --git a/report/sets.go b/report/sets.go index 183ada1deb..59e67270cb 100644 --- a/report/sets.go +++ b/report/sets.go @@ -1,10 +1,7 @@ package report import ( - "bytes" - "fmt" "reflect" - "sort" "github.com/ugorji/go/codec" "github.com/weaveworks/ps" @@ -109,22 +106,7 @@ func (s Sets) Copy() Sets { } func (s Sets) String() string { - if s.psMap == nil { - s = EmptySets - } - keys := []string{} - for _, k := range s.psMap.Keys() { - keys = append(keys, k) - } - sort.Strings(keys) - - buf := bytes.NewBufferString("{") - for _, key := range keys { - val, _ := s.psMap.Lookup(key) - fmt.Fprintf(buf, "%s: %v, ", key, val) - } - fmt.Fprintf(buf, "}") - return buf.String() + return mapToString(s.psMap) } // DeepEqual tests equality with other Sets From 29f2af11d90ac5c185d80447840bf58274f3014b Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 29 May 2017 13:48:31 +0100 Subject: [PATCH 3/3] introduce mapKeys helper --- report/counters.go | 22 ++++------------------ report/map_helpers.go | 19 ++++++++++--------- report/node_set.go | 30 +++--------------------------- 3 files changed, 17 insertions(+), 54 deletions(-) diff --git a/report/counters.go b/report/counters.go index a110330dd7..660c32af4a 100644 --- a/report/counters.go +++ b/report/counters.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "reflect" - "sort" "github.com/ugorji/go/codec" "github.com/weaveworks/ps" @@ -87,28 +86,15 @@ func (c Counters) Merge(other Counters) Counters { return Counters{output} } -// ForEach calls f for each k/v pair of counters. Keys are iterated in -// lexicographical order. -func (c Counters) ForEach(f func(key string, val int)) { - if c.psMap != nil { - keys := c.psMap.Keys() - sort.Strings(keys) - for _, key := range keys { - if val, ok := c.psMap.Lookup(key); ok { - f(key, val.(int)) - } - } - } -} - // String serializes Counters into a string. func (c Counters) String() string { buf := bytes.NewBufferString("{") prefix := "" - c.ForEach(func(k string, v int) { - fmt.Fprintf(buf, "%s%s: %d", prefix, k, v) + for _, key := range mapKeys(c.psMap) { + val, _ := c.psMap.Lookup(key) + fmt.Fprintf(buf, "%s%s: %d", prefix, key, val.(int)) prefix = ", " - }) + } fmt.Fprintf(buf, "}") return buf.String() } diff --git a/report/map_helpers.go b/report/map_helpers.go index f42978af4a..3d6bc7a057 100644 --- a/report/map_helpers.go +++ b/report/map_helpers.go @@ -63,16 +63,8 @@ func mapEqual(m, n ps.Map, equalf func(a, b interface{}) bool) bool { // very similar to ps.Map.String() but with keys sorted func mapToString(m ps.Map) string { - keys := []string{} - if m != nil { - for _, k := range m.Keys() { - keys = append(keys, k) - } - sort.Strings(keys) - } - buf := bytes.NewBufferString("{") - for _, key := range keys { + for _, key := range mapKeys(m) { val, _ := m.Lookup(key) fmt.Fprintf(buf, "%s: %s,\n", key, val) } @@ -80,6 +72,15 @@ func mapToString(m ps.Map) string { return buf.String() } +func mapKeys(m ps.Map) []string { + if m == nil { + return nil + } + keys := m.Keys() + sort.Strings(keys) + return keys +} + // constants from https://github.com/ugorji/go/blob/master/codec/helper.go#L207 const ( containerMapKey = 2 diff --git a/report/node_set.go b/report/node_set.go index b302488a29..5a749ca18b 100644 --- a/report/node_set.go +++ b/report/node_set.go @@ -3,7 +3,6 @@ package report import ( "bytes" "fmt" - "sort" "github.com/davecgh/go-spew/spew" "github.com/ugorji/go/codec" @@ -85,16 +84,6 @@ func (n NodeSet) Lookup(key string) (Node, bool) { return Node{}, false } -// Keys is a list of all the keys in this set. -func (n NodeSet) Keys() []string { - if n.psMap == nil { - return nil - } - k := n.psMap.Keys() - sort.Strings(k) - return k -} - // Size is the number of nodes in the set func (n NodeSet) Size() int { if n.psMap == nil { @@ -106,7 +95,7 @@ func (n NodeSet) Size() int { // ForEach executes f for each node in the set. Nodes are traversed in sorted // order. func (n NodeSet) ForEach(f func(Node)) { - for _, key := range n.Keys() { + for _, key := range mapKeys(n.psMap) { if val, ok := n.psMap.Lookup(key); ok { f(val.(Node)) } @@ -119,22 +108,9 @@ func (n NodeSet) Copy() NodeSet { } func (n NodeSet) String() string { - keys := []string{} - if n.psMap == nil { - n = EmptyNodeSet - } - psMap := n.psMap - if psMap == nil { - psMap = ps.NewMap() - } - for _, k := range psMap.Keys() { - keys = append(keys, k) - } - sort.Strings(keys) - buf := bytes.NewBufferString("{") - for _, key := range keys { - val, _ := psMap.Lookup(key) + for _, key := range mapKeys(n.psMap) { + val, _ := n.psMap.Lookup(key) fmt.Fprintf(buf, "%s: %s, ", key, spew.Sdump(val)) } fmt.Fprintf(buf, "}")