Refactor copy of slices and maps #2128
Conversation
Honny1
force-pushed
the
refactor-copy-map
branch
2 times, most recently
from
a9ef2aa
to
13a96fe
Compare
store.go
Outdated
| options.BigData = copyContainerBigDataOptionSlice(cOptions.BigData) | ||
| options.BigData = slices.Clone(cOptions.BigData) |
There was a problem hiding this comment.
slices.Clone makes a shallow clone, i.e. Data slice is not really copied with this change.
There was a problem hiding this comment.
The previous copyContainerBigDataOptionSlice function used the slices.Clone function to copy data. So I would expect similar behavior.
There was a problem hiding this comment.
It used slices.Clone() to copy the Data field in each element in the slice. The godoc for slices.Clone() notes that it only copies using assignment, so a new slice would not be created for Data fields.
There was a problem hiding this comment.
Okay, I've reverted these changes.
|
/approved |
Honny1
force-pushed
the
refactor-copy-map
branch
from
13a96fe
to
b1a1078
Compare
Removes duplicate copy*Slice functions using a generic copy function or replaces them with the slices.Clone function. Also simplifies the stringSliceWithoutValue function. These changes should not change the behavior. Signed-off-by: Jan Rodák <[email protected]>
Removes the duplicate copy*Map function using the general function newMapFrom. Reduces the allocation of empty maps using the copyMapPrefferingNil function. This change may affect the behavior so that instead of an empty allocated map, a nil will be returned. Signed-off-by: Jan Rodák <[email protected]>
Honny1
force-pushed
the
refactor-copy-map
branch
from
b1a1078
to
43d0009
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Honny1, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM |
|
/lgtm |
This PR refactors:
slices.Clonefunction.stringSliceWithoutValuefunction.newMapFrom.copyMapPrefferingNilfunction.nilwill be returned.Follows up on: #2122