6

I don't understand the behavior of the following piece of code. In creating a list of matching structs as a slice of struct pointers, the code always prints the last element of original array (which actually wasn't a match)—it prints 12 and 12. However, if I change matches to be []Widget instead of []*Widget, then it will print 10 and 11.

Why is this?

package main

import (
    "fmt"
)

func main() {

    type Widget struct {
        id    int
        attrs []string
    }

    widgets := []Widget{
        Widget{
            id:    10,
            attrs: []string{"blah", "foo"},
        },
        Widget{
            id:    11,
            attrs: []string{"foo", "bar"},
        },
        Widget{
            id:    12,
            attrs: []string{"xyz"},
        },
    }

    var matches []*Widget
    for _, w := range widgets {
        for _, a := range w.attrs {
            if a == "foo" {
                matches = append(matches, &w)
                break
            }
        }
    }

    for _, m := range matches {
        fmt.Println(m.id)
    }

}
eblood
  • 105
  • 2
  • 6

1 Answers1

8

That's because when you use the pointers you are adding &w to the array.

Note that w is actually the local variable used in the loop, so that's not the address you want to add to the matches array.

(even though the value of the variable w changes through the loop, its address stays the same)

When the loop ends, w ends up with the last value so that's why it prints 12 two times.

You need to add the address of the element that matched instead.

If you do this:

matches = append(matches, &widgets[i])

Then it'd work fine with pointers as well.

Modified Go playground for you to test it:

https://play.golang.org/p/YE-cokyEHu

cmaher
  • 5,100
  • 1
  • 22
  • 34
eugenioy
  • 11,825
  • 28
  • 35
  • This is one of the reasons I just always prepend `&` when I create structs. No weird headache cases like this arise. I'll create a struct value every now and again only if I'm sure I want it on the stack. – RayfenWindspear Aug 02 '17 at 18:26
  • Ah, that makes sense. Thanks! – eblood Aug 02 '17 at 18:27
  • 1
    @RayfenWindspear I think it's more reasonable to try to understand something and it's implications in different scenarios rather than just using a generic try to catch all solution because always using pointers also has its drawbacks, not only performance wise but logically – Jimmy T. Dec 25 '20 at 22:01
  • @jimmy-t Very true. I wrote that comment 3 years ago and have learned a lot since. The nuances and considerations you are referring to warrant a blog post sized response, which isn't appropriate for this type of question. But ultimately, no, I shouldn't have made a blanket statement like that. The only way to know is to understand the mechanics enough to make an educated decision. In short, Go is copy by value, so copying large structs without pointers isn't ideal, nor is allocating a million tiny throwaway structs on the heap using pointers that have to be garbage collected. – RayfenWindspear Dec 29 '20 at 02:07