10

I was expecting to see 3, what's going on?

package main

import "fmt"

type Counter struct {
    count int
}

func (self Counter) currentValue() int {
    return self.count
}
func (self Counter) increment() {
    self.count++
}

func main() {
    counter := Counter{1}
    counter.increment()
    counter.increment()

    fmt.Printf("current value %d", counter.currentValue())
}

http://play.golang.org/p/r3csfrD53A

OscarRyz
  • 196,001
  • 113
  • 385
  • 569
  • 2
    Pretty much a duplicate of http://stackoverflow.com/questions/16540481/why-is-this-struct-not-working – nemo May 17 '13 at 19:27

2 Answers2

30

Your method receiver is a struct value, which means the receiver gets a copy of the struct when invoked, therefore it's incrementing the copy and your original isn't updated.

To see the updates, put your method on a struct pointer instead.

func (self *Counter) increment() {
    self.count++
}

Now self is a pointer to your counter variable, and so it'll update its value.

http://play.golang.org/p/h5dJ3e5YBC

  • 1
    @OscarRyz you should tag this as the answer if it solved your problem. – Lander May 18 '13 at 04:37
  • Correct me if I am wrong, but using the first character of the type is pretty much the 'convention' in go (so not calling it 'self'). However, self makes a lot of sense in the pointer scenario. –  Jun 24 '14 at 23:13
  • 2
    @Corey: I always use `self` so that I never have to try to interpret what the identifier is referencing. I always know it's the value on which the method was invoked. I never found much merit in having a different name for each different type. –  Jul 01 '14 at 19:52
1

I want to add to @user1106925 response.

If you need to use the custom type inside a map you need to use a map to pointer. because the for l,v :=range yourMapType will receive a copy of the struct.

Here a sample:

package main

import "fmt"

type Counter struct {
        Count int
}

func (s *Counter) Increment() {
        s.Count++
}

func main() {
        // Using map to type
        m := map[string]Counter{
                "A": Counter{},
        }

        for _, v := range m {
                v.Increment()
        }
        fmt.Printf("A: %v\n", m["A"].Count)

        // Now using map to pointer
        mp := map[string]*Counter{
                "B": &Counter{},
        }

        for _, v := range mp {
                v.Increment()
        }
        fmt.Printf("B: %v\n", mp["B"].Count)
}

The output is:

$ go build && ./gotest
A: 0
B: 1
ton
  • 3,827
  • 1
  • 42
  • 40
  • beware, you can also work by copy. There is no mandatory obligation to use a map of pointers like `map[string]*Counter`. https://goplay.space/#uZ9mD8KmZL4 –  Aug 14 '21 at 10:25
  • Agreed, but you can have parallelism issues by using copy, because another tasks can increment another copy and you can lost counters – ton Aug 15 '21 at 15:29
  • considering concurrent access, using `map[string]*Counter` allows for the Counter type to embed the `sync.Lock` behavior, which in turns allow for row locked access. Using `map[string]Counter` you could not embed `sync.Lock`, thus you would have to lock at the storage level. The latter might become a source of contention. –  Aug 15 '21 at 15:35
  • Here a sample issue using copy inside map instead of pointers: https://goplay.space/#7nLjdiQkvTU – ton Aug 15 '21 at 15:58
  • take care, this code is invalid. https://play.golang.org/p/lyI7r9g72R4 –  Aug 15 '21 at 16:01
  • Thank you for pointing out the data races. But I don't think this disqualifies the code fragment. The point of that PoC code is to clearly show the issue using copies inside maps. And yet with the data races, the map pointer was stable and the copy was not. – ton Aug 15 '21 at 16:12
  • 1
    sure, you are free to subordinate hypothesis on top of invalid program. although, note that you are being lucky that the calls to `time.Sleep(time.Duration(rand.Int()%400) * time.Millisecond)` interleaves the write operations at `v.Increment()` Those are poorly parallelized; –  Aug 15 '21 at 16:21
  • 1
    mk-cbon, until now I was unable to reproduce the issue without data-races. So I do agree that a well written code works with copy in maps too. – ton Aug 15 '21 at 17:32