How to use pointers or indexing on a range to resolve rangeValCopy gocritic message

Question

I'm getting the following output when running https://golangci-lint.run/:

rangeValCopy: each iteration copies 128 bytes (consider pointers or indexing) (gocritic)
    for _, v := range products {

Here is a cut down version of the code I am running:

package main

import (
    "fmt"
    "encoding/json"
)

type Application struct {
    ProductData   []ProductDatum
}

type ProductDatum struct {
    Name          string
    ProductBrand  string
    ProductType   string
}

type Item struct {
    ProductBrand string
    ProductName  string
    ProductType  string
}

func main() {
    appl := Application{
        ProductData: []ProductDatum{
            {
                Name:         "Baz",
                ProductBrand: "Foo",
                ProductType:  "Bar",
            },
        },
    }

    products := appl.ProductData

    var orderLinesItem []Item

    for _, v := range products {
        item := []Item{
            {
                ProductBrand: v.ProductBrand,
                ProductName:  v.Name,
                ProductType:  v.ProductType,
            },
        }

        orderLinesItem = append(orderLinesItem, item...)
    }
    
    
    body, _ := json.Marshal(orderLinesItem)
    
    fmt.Println(string(body))
}

Here it is in go playground.

What does this output mean and how can I do what it's asking? I tried to use a pointer on each item but that didn't seem to make a difference.

Change `ProductData []ProductDatum` to `ProductData []*ProductDatum`. — mkopriva, Mar 17 '21 at 20:44
The [pointers](https://play.golang.org/p/E71aoVNaMtZ) version. The [indexing](https://play.golang.org/p/MMOEoacQZgh) version. — mkopriva, Mar 17 '21 at 20:46
FYI You have `for _, v := range products { ...` and there the `v` is the `rangeVal` being hinted to by the `rangeValCopy: ...` message. — mkopriva, Mar 17 '21 at 20:51

Pablo Flores · Accepted Answer · 2021-03-18T16:50:15.747

What the linter is trying to tell you is that, by using range the way you are using it, each time you get a new element v it is not returning an element directly from the collection, rather it's a new copy of that element. The linter suggest two approaches: Either change the slice to be a slice of pointers to struct, that way each iteration of the for loop will get a reference to an element instead of a full struct copy

var products []*ProductDatum
//fill products slice

var orderLinesItem []Item

for _, v := range products{
  //here v is a pointer instead of a full copy of a struct. 
  //Go dereferences the pointer automatically therefore you don't have to use *v
  item := []Item{
            {
                ProductBrand: v.ProductBrand,
                ProductName:  v.Name,
                ProductType:  v.ProductType,
            },
        } 
}

The other suggestion from the linter is using the index value that the range returns on each iteration

for i := range products{
   item := []Item{
            {
                //access elements by index directly
                ProductBrand: products[i].ProductBrand,
                ProductName:  products[i].Name,
                ProductType:  products[i].ProductType,
            },
        } 
}

How to use pointers or indexing on a range to resolve rangeValCopy gocritic message

1 Answers1