Skip to content

Commit

Permalink
shallow-copy instead of (deep) Copy() in Merge()
Browse files Browse the repository at this point in the history
Merge() must not modify self or other; shallow-copying is sufficient
to achieve that.
  • Loading branch information
rade committed Jul 29, 2016
1 parent c4fa721 commit 0304f50
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
11 changes: 7 additions & 4 deletions report/metadata_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,14 @@ func (e MetadataTemplates) Copy() MetadataTemplates {
// Merge merges two sets of MetadataTemplates so far just ignores based
// on duplicate id key
func (e MetadataTemplates) Merge(other MetadataTemplates) MetadataTemplates {
result := e.Copy()
if e == nil && other == nil {
return nil
}
result := make(MetadataTemplates, len(e))
for k, v := range e {
result[k] = v
}
for k, v := range other {
if result == nil {
result = MetadataTemplates{}
}
if existing, ok := result[k]; !ok || existing.Priority < v.Priority {
result[k] = v
}
Expand Down
11 changes: 7 additions & 4 deletions report/metric_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,14 @@ func (e MetricTemplates) Copy() MetricTemplates {
// Merge merges two sets of MetricTemplates so far just ignores based
// on duplicate id key
func (e MetricTemplates) Merge(other MetricTemplates) MetricTemplates {
result := e.Copy()
if e == nil && other == nil {
return nil
}
result := make(MetricTemplates, len(e))
for k, v := range e {
result[k] = v
}
for k, v := range other {
if result == nil {
result = MetricTemplates{}
}
if existing, ok := result[k]; !ok || existing.Priority < v.Priority {
result[k] = v
}
Expand Down
5 changes: 4 additions & 1 deletion report/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,10 @@ func (n Nodes) Copy() Nodes {
// Merge merges the other object into this one, and returns the result object.
// The original is not modified.
func (n Nodes) Merge(other Nodes) Nodes {
cp := n.Copy()
cp := make(Nodes, len(n))
for k, v := range n {
cp[k] = v
}
for k, v := range other {
if n, ok := cp[k]; ok { // don't overwrite
v = v.Merge(n)
Expand Down

0 comments on commit 0304f50

Please sign in to comment.