I'm struggling to understand why I have a bug in my code in one state but not the other. It's been a while since I've covered pointers, so I'm probably rusty!
Basically I have a repository structure I'm using to store an object in memory, that has a Store
function.
type chartsRepository struct { mtx sync.RWMutex charts map[ChartName]*Chart } func (r *chartsRepository) Store(c *Chart) error { r.mtx.Lock() defer r.mtx.Unlock() r.charts[c.Name] = c return nil }
So all it does is put a RW mutex lock on and adds the pointer to a map, referenced by an identifier.
Then I've got a function that will basically loop through a slice of these objects, storing them all in the repository.
type service struct { charts Repository } func (svc *service) StoreCharts(arr []Chart) error { hasError := false for _, chart := range arr { err := svc.repo.Store(&chart) // ... error handling } if hasError { // ... Deals with the error object return me } return nil }
The above doesn't work, it looks like everything works fine at first, but on trying to access the data later, the entries in the map all point to the same Chart
object, despite having different keys.
If I do the following and move the pointer reference to another function, everything works as expected:
func (svc *service) StoreCharts(arr []Chart) error { // ... for _, chart := range arr { err := svc.storeChart(chart) } // ... } func (svc *service) storeChart(c Chart) error { return svc.charts.Store(&c) }
I'm assuming the issue is that because the loop overwrites the reference to the chart
in the for
loop, the pointer reference also changes. When the pointer is generated in an independent function, that reference is never overwritten. Is that right?
I feel like I'm being stupid, but shouldn't the pointer be generated by &chart
and that's independent of the chart
reference? I also tried creating a new variable for the pointer p := &chart
in the for
loop and that didn't work either.
Should I just avoid generating pointers in loops?
we assign the pointer to the array str to p. In C the following assignments have the same effect: p = &str[0]; p = str; “By definition, the value of a variable or expression of type array is the address of element zero of the array” (K & R (2)).
This is done as follows. int *ptr = &arr[0]; After this, a for loop is used to dereference the pointer and print all the elements in the array. The pointer is incremented in each iteration of the loop i.e at each loop iteration, the pointer points to the next element of the array.
In simple words, array names are converted to pointers. That's the reason why you can use pointers to access elements of arrays. However, you should remember that pointers and arrays are not the same. There are a few cases where array names don't decay to pointers.
This is because there is only a single loop variable chart
, and in each iteration just a new value is assigned to it. So if you attempt to take the address of the loop variable, it will be the same in each iteration, so you will store the same pointer, and the pointed object (the loop variable) is overwritten in each iteration (and after the loop it will hold the value assigned in the last iteration).
This is mentioned in Spec: For statements: For statements with range
clause:
The iteration variables may be declared by the "range" clause using a form of short variable declaration (
:=
). In this case their types are set to the types of the respective iteration values and their scope is the block of the "for" statement; they are re-used in each iteration. If the iteration variables are declared outside the "for" statement, after execution their values will be those of the last iteration.
Your second version works, because you pass the loop variable to a function, so a copy will be made of it, and then you store the address of the copy (which is detached from the loop variable).
You can achieve the same effect without a function though: just create a local copy and use the address of that:
for _, chart := range arr { chart2 := chart err := svc.repo.Store(&chart2) // Address of the local var // ... error handling }
Also note that you may also store the address of the slice elements:
for i := range arr { err := svc.repo.Store(&arr[i]) // Address of the slice element // ... error handling }
The disadvantage of this is that since you store pointers to the slice elements, the whole backing array of the slice would have to be kept in memory for as long as you keep any of the pointers (the array cannot be garbage collected). Moreover, the pointers you store would share the same Chart
values as the slice, so if someone would modify a chart value of the passed slice, that would effect the charts whose pointers you stored.
See related questions:
Golang: Register multiple routes using range for loop slices/map
Why do these two for loop variations give me different behavior?
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With