Refactor Overwriting Combinations in Matrix

EmmaMunley · EmmaMunley · commit 6593acf1128e · 2023-03-29T13:01:16.000-04:00
This commit refactors the overwriting combinations function within fanning out the Matrix to reduce the number of calls as requested by Priti in PR: tektoncd#6341.
diff --git a/pkg/apis/pipeline/v1/matrix_types.go b/pkg/apis/pipeline/v1/matrix_types.go
@@ -72,7 +72,7 @@ func (m *Matrix) FanOut() []Params {
 	for _, parameter := range m.Params {
 		combinations = combinations.fanOutMatrixParams(parameter)
 	}
-	combinations = combinations.overwriteCombinations(includeCombinations)
+	combinations.overwriteCombinations(includeCombinations)
 	combinations = combinations.addNewCombinations(includeCombinations)
 	return combinations.toParams()
 }
@@ -84,7 +84,11 @@ func (cs Combinations) overwriteCombinations(ics Combinations) Combinations {
 	for _, paramCombination := range cs {
 		for _, includeCombination := range ics {
 			if paramCombination.contains(includeCombination) {
-				includeCombination.overwrite(paramCombination)
+				// overwrite the parameter name and value in existing combination
+				// with the include combination
+				for name, val := range includeCombination {
+					paramCombination[name] = val
+				}
 			}
 		}
 	}
@@ -115,14 +119,6 @@ func (c Combination) contains(includeCombination Combination) bool {
 	return true
 }
 
-// overwrite the parameter name and value exists in combination with the include combination
-func (c Combination) overwrite(oldCombination Combination) Combination {
-	for name, val := range c {
-		oldCombination[name] = val
-	}
-	return oldCombination
-}
-
 // shouldAddNewCombination returns true if the include parameter name exists but the value is
 // missing from combinations
 func (cs Combinations) shouldAddNewCombination(includeCombination map[string]string) bool {
diff --git a/pkg/apis/pipeline/v1beta1/matrix_types.go b/pkg/apis/pipeline/v1beta1/matrix_types.go
@@ -72,7 +72,7 @@ func (m *Matrix) FanOut() []Params {
 	for _, parameter := range m.Params {
 		combinations = combinations.fanOutMatrixParams(parameter)
 	}
-	combinations = combinations.overwriteCombinations(includeCombinations)
+	combinations.overwriteCombinations(includeCombinations)
 	combinations = combinations.addNewCombinations(includeCombinations)
 	return combinations.toParams()
 }
@@ -84,7 +84,11 @@ func (cs Combinations) overwriteCombinations(ics Combinations) Combinations {
 	for _, paramCombination := range cs {
 		for _, includeCombination := range ics {
 			if paramCombination.contains(includeCombination) {
-				includeCombination.overwrite(paramCombination)
+				// overwrite the parameter name and value in existing combination
+				// with the include combination
+				for name, val := range includeCombination {
+					paramCombination[name] = val
+				}
 			}
 		}
 	}
@@ -115,14 +119,6 @@ func (c Combination) contains(includeCombination Combination) bool {
 	return true
 }
 
-// overwrite the parameter name and value exists in combination with the include combination
-func (c Combination) overwrite(oldCombination Combination) Combination {
-	for name, val := range c {
-		oldCombination[name] = val
-	}
-	return oldCombination
-}
-
 // shouldAddNewCombination returns true if the include parameter name exists but the value is
 // missing from combinations
 func (cs Combinations) shouldAddNewCombination(includeCombination map[string]string) bool {

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ func (m *Matrix) FanOut() []Params {`
`72`	`72`	`for _, parameter := range m.Params {`
`73`	`73`	`combinations = combinations.fanOutMatrixParams(parameter)`
`74`	`74`	`}`
`75`		`- combinations = combinations.overwriteCombinations(includeCombinations)`
	`75`	`+ combinations.overwriteCombinations(includeCombinations)`
`76`	`76`	`combinations = combinations.addNewCombinations(includeCombinations)`
`77`	`77`	`return combinations.toParams()`
`78`	`78`	`}`
`@@ -84,7 +84,11 @@ func (cs Combinations) overwriteCombinations(ics Combinations) Combinations {`
`84`	`84`	`for _, paramCombination := range cs {`
`85`	`85`	`for _, includeCombination := range ics {`
`86`	`86`	`if paramCombination.contains(includeCombination) {`
`87`		`- includeCombination.overwrite(paramCombination)`
	`87`	`+ // overwrite the parameter name and value in existing combination`
	`88`	`+ // with the include combination`
	`89`	`+ for name, val := range includeCombination {`
	`90`	`+ paramCombination[name] = val`
	`91`	`+ }`
`88`	`92`	`}`
`89`	`93`	`}`
`90`	`94`	`}`
`@@ -115,14 +119,6 @@ func (c Combination) contains(includeCombination Combination) bool {`
`115`	`119`	`return true`
`116`	`120`	`}`
`117`	`121`
`118`		`-// overwrite the parameter name and value exists in combination with the include combination`
`119`		`-func (c Combination) overwrite(oldCombination Combination) Combination {`
`120`		`- for name, val := range c {`
`121`		`- oldCombination[name] = val`
`122`		`- }`
`123`		`- return oldCombination`
`124`		`-}`
`125`		`-`
`126`	`122`	`// shouldAddNewCombination returns true if the include parameter name exists but the value is`
`127`	`123`	`// missing from combinations`
`128`	`124`	`func (cs Combinations) shouldAddNewCombination(includeCombination map[string]string) bool {`