Using structs, pointers, and maps

[Boris Solovyov]

Hi list,

If I make a map of structs, which have methods with side effects, it looks like I must store pointers to the structs in the map, not store the structs themselves. Because you can't take pointer of item in map to call a pointer-reciever method on it, and if you don't define method with pointer receiver, side effects change copy of the struct, not the struct itself.

Example: http://play.golang.org/p/Uw0Exar90q

I am just checking to see that I am not missing anything. This is The Right Way To Do It, right? There is not some other way that is better Go code?

Thanks,

Boris

[bryanturley]

"
// I can do it this way, but I kind of wish I could avoid pointers
"
There are no variables when you get to the lowest level of code only registers, immediates (literals-ish) and pointers.

Pointer arithmetic is gone so pointers are much less error prone in go.

If you take the address of the inside of a map and then the map has to grow your object won't be at that address anymore.
That is why you should store a pointer to your object that won't move as in your final example.
The maps copy of the pointer can move around randomly and your object is still located at the address your pointer is pointing to.

 

[Daniel Bryan]

I've written programs that deal with maps of large numbers of both structs and slices and this is how I've always done it. Even if you could store the struct directly on the map instead of a pointer, you'd have to change your design if you ever wanted to initialise a struct via some constructor function (very common when the struct type comes from another package).

[Cole Mickens]

There's really no way around it. If you're taking the value out, you can put it back in and reassign m["me"] like so: http://play.golang.org/p/T7i4uElKd2

I'd normally just use pointers, but I don't know what your use case is.

On Thursday, December 27, 2012 2:48:06 PM UTC-6, Boris wrote:

[Dave Collins]

On Saturday, January 12, 2013 11:57:11 AM UTC-6, taric...@gmail.com wrote:

I have the same question, but using interface pointers.  I've been having difficulty getting the following to work:

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

Your mistake here is trying to store a pointer to an interface in the map.  The map is still storing pointers to your struct at that point.

I've updated your example here:  http://play.golang.org/p/wwyLmEVMTE

However, if you really wanted to store pointers to interfaces (which I can't see why you would really want to), you'd have to stuff the value into an interface and set the map entry to the address of the interface.  On the way out, you'd have to reverse it.  That is to say get the pointer to the interface out, dereference it, and do a type assertion to your *S on that interface, *not* on the *interface as you were trying to do in your example.

I think what you really want though is the correction I provided above.

[Dave Collins]

On Saturday, January 12, 2013 1:00:02 PM UTC-6, Dave Collins wrote:

The map is still storing pointers to your struct at that point.

 
This statement wasn't clear.  What I meant is that the map is still storing pointers to your struct when using an ordinary interface.  Think of it this way:

Normal interface containing an *S:

interface type: *S
interface value: 0x123123123 (whatever the address of the S struct is)

By doing the *interface, you are adding another layer of indirection.  So it would be the address of the interface that contains *S as illustrated above.

[Nate Finch]

#1 - pointers are your friend. Don't be afraid of them. Most of the time you'll be dealing with pointers to structs. It's no big deal.

#2 - interfaces are effectively like pointers already, so you don't generally need/want to use pointers to interfaces.

[Kevin Gillette]

On Saturday, January 12, 2013 12:26:18 PM UTC-7, Nate Finch wrote:

#1 - pointers are your friend. Don't be afraid of them. Most of the time you'll be dealing with pointers to structs. It's no big deal.

My strategy with structs and is method receivers is: if the value is large (16 words or more) or must have caller-visible changes, use a pointer. Otherwise, use your discretion (the performance cost of a 10 word copy, say, vs a pointer indirection, may well favor the copy, yet is likely still negligible).

 

#2 - interfaces are effectively like pointers already, so you don't generally need/want to use pointers to interfaces.

 The first part is only true semantically if pointer values are being stored in the interface. While a sufficiently large non-pointer value will have to be pointed to by the interface header (rather than stored inline), that will be a pointer to a copy of the original, and therefore cannot be used to recover the address of the original.

But yes, there are very few times you'll need a pointer to an interface value, and only ever when you already know that's exactly what you need -- I strongly doubt that interface pointers can be "stumbled onto" as the correct solution for a given problem.

[Taric Mirza]

Thanks! Works like a charm and is helping cleaning up my code a ton.

One other question, this is really more about coding style:

In the case where you manipulate members of the struct, then using
pointers as in your example is the way to go.

But, you have a choice for functions that just read values from the
struct instead of manipulating it. Is there a best practice coding
style here, between dereferencing the struct and then using that, or
dereferencing each member of the struct as you go? eg:

// A:

laser := worldobj.(*Laser)
fmt.Printf("%0.4f,%0.4f", (*laser).x, (*laser).y)

versus

// B:

laser := *(worldobj.(*Laser))
fmt.Printf("%0.4f,%0.4f", laser.x, laser.y)


I'm kind of torn. I would imagine A) has slightly better
performance, and doesn't require any code-rework if you later on need
to manipulate the struct.

On the other hand, B) is more readable since you don't have to look at
pointers all over the place, just on one line.

[Dave Collins]

On Saturday, January 12, 2013 3:52:35 PM UTC-6, Taric Mirza wrote:

Thanks!  Works like a charm and is helping cleaning up my code a ton.

One other question, this is really more about coding style:

In the case where you manipulate members of the struct, then using
pointers as in your example is the way to go.

But, you have a choice for functions that just read values from the
struct instead of manipulating it.  Is there a best practice coding
style here, between dereferencing the struct and then using that, or
dereferencing each member of the struct as you go?  eg:

// A:

laser := worldobj.(*Laser)
fmt.Printf("%0.4f,%0.4f", (*laser).x, (*laser).y)

versus

// B:

laser := *(worldobj.(*Laser))
fmt.Printf("%0.4f,%0.4f", laser.x, laser.y)


I'm kind of torn.   I would imagine A) has slightly better
performance, and doesn't require any code-rework if you later on need
to manipulate the struct.

On the other hand, B) is more readable since you don't have to look at
pointers all over the place, just on one line.

Actually, you don't need to dereference at all.  Go automatically handles this for you.

See this example:  http://play.golang.org/p/ANaKaFSQLn

[Taric Mirza]

Oh wow, nice!

Thanks so much. The more I learn about Go the more I realize how
simple and elegant they made it.

[Kevin Gillette]

Indeed. In addition to implicit dereferencing for value receivers, the reverse also works as well: anything that is addressable (including 'value' variables on the stack, or a field of element of anything that's addressable) will implicitly be addressed when a pointer-receiver method is called on them (though you must explicitly use the address operator when you need to pass value variables as pointers).

There is a consideration to make, though: historically it has been considered bad form in Go to give a type a mix of value and pointer receivers in methods without a very specific reason for doing so. The typical justification is that a small struct in a getter method might as well have a value receiver even though the corresponding setter method uses a pointer receiver; this, however, can lead to confusion on the part of the app programmer if they start out using only the read-only methods upon what turns out to be a value-copy of the original (but hey, it compiled and seems to work, so it must be correct) -- when use of pointer-receiver methods don't seem to produce the documented changes in the original, it can be difficult to debug.

[Ken Lee]

---

There is a consideration to make, though: historically it has been considered bad form in Go to give a type a mix of value and pointer receivers in methods without a very specific reason for doing so.

---

Is this still the case now? As in 2024.

[Ian Lance Taylor]

On Mon, Oct 7, 2024 at 10:29 AM Ken Lee <ken.lee....@gmail.com> wrote:
>
> ---
> There is a consideration to make, though: historically it has been considered bad form in Go to give a type a mix of value and pointer receivers in methods without a very specific reason for doing so.
> ---
>
> Is this still the case now? As in 2024.

As a general guideline, yes.

https://go.dev/wiki/CodeReviewComments#receiver-type

Ian

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/03df7dce-5c48-44a3-bc3c-851ded2a1f08n%40googlegroups.com.

[Axel Wagner]

To be honest, I always found this recommendation a little bit strange, personally.

I'll note that the standard library does not really keep to this either. For example, time.Time.UnmarshalText (obviously) has a pointer-receiver, while almost all other methods on time.Time have a value receiver.

And if you implement flag.Value, the Set method obviously needs a pointer receiver, but if the String method has one as well, it won't print properly when used as a value. In basically every implementation of flag.Value I've ever written, String needed a value receiver, while Set needed a pointer receiver.

I understand the basic idea of the advice, that if a type keeps state that is manipulated via methods, then it should generally be passed around as a pointer, so giving all the methods a pointer-receiver works well. But if a type *is* intended to be used as a value (like time.Time or Enum in my example) then you will almost certainly end up with a mix of receiver kinds - as soon as you want to add any form of de-serialization to it. So "don't mix receiver kinds" seems like misleading advice to me.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAOyqgcX7v9Edk5beRH38tfJO18ZUXv-nOHsEPPCfMQy0hz%3DFdw%40mail.gmail.com.

[burak serdar]

Mixing pointer and value receivers can be race-prone, because of the
copying involved in passing value receivers.

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfGcq2nxaik_qAWoX81W-tTKRRYBDM5_6%3DefSv4tr8b03g%40mail.gmail.com.

[Cleberson Pedreira Pauluci]

Many places and books I've read generally say: If a function needs to update a variable, or if an argument is so large that we want to avoid copying it, we should pass the pointer. Same for methods (pointer receiver). (The Go programming language book).

About mixing "value receiver" and "pointer receiver". Even the IDE complains about this and recommends following the Go documentation. (Goland)

[Axel Wagner]

No offence, but I made an argument. You don't have to agree with the argument and it might be wrong. But to convince me, at least, that argument would need to actually be referenced.

I gave reasons why, in my opinion, *not* mixing value and pointer receivers sometimes leads to incorrect code. So as far as I'm concerned (until someone tells me my reasons are wrong) Goland's linter simply encourages you to write bad code. It would not be the first time that I strongly disagree with the recommendations of an IDE. Goland in particular has a history of making, in my opinion, pretty questionable decisions.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9b28006b-c310-417e-9afc-e7f5c470641cn%40googlegroups.com.

[Robert Engels]

I am pretty sure it is immaterial. If the object isn’t immutable any copy or mutation operation needs to be synchronized. 

But the problem afaik is that you can’t control synchronization when the object is copied for a value receiver - which means you cant properly synchronize when you have pointer and value receivers unless you do it externally (which is a huge pain to do everywhere). 

On Oct 7, 2024, at 4:43 PM, 'Axel Wagner' via golang-nuts <golan...@googlegroups.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfEj%3DQACB31VMc7ami7xt9tMF00kYxFUfZpWfZ0j65GWsw%40mail.gmail.com.

[Axel Wagner]

My argument had nothing to do with synchronization.

FTR I find the synchronization argument also extremely dubious. By that argument, you also can't pass the address to a local variable to another function, when using it as a value elsewhere. It's a weird argument to make. time.Time uses a mix of pointer- and value receivers and IMO no one can make a serious argument that this would expose programs to risks of data races.

But to repeat my actual argument in favour of (sometimes) mixing receiver kinds:

1. It is totally reasonable to use some types as values.

2. Such types, intended to be used as values, will need to use value-receivers for some methods, as otherwise their value-version does not implement certain interfaces (methods are not promoted from pointer to value types). Like fmt.Stringer, for example. And

3. such types still need to sometimes use pointer-receivers, to implement functionalities like unmarshalling.

time.Time is a standard library example of such a type. I also provided an example for an "enum-like" type implementing flag.Value.

[Robert Engels]

I am fairly certain if you mix pointer and receiver methods and the receiver methods mutate - even if you synchronize those you will get a data race calling the value methods. It must afaik as the runtime/compiler has no implicit synchronization when creating the copies. That is a data race. 

On Oct 7, 2024, at 5:10 PM, Axel Wagner <axel.wa...@googlemail.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfFYZ1DTD9fTVzNHtOp7Ed7w3_x8QbxsB2x_%2BTs%3DtxY0BA%40mail.gmail.com.

[robert engels]

I wrote a simple test. Sure enough it fails, and it reports a data race.

package main

import (
    "log"
    "sync"
)

type S struct {
    sync.Mutex
    index int
    values [128]int
}

func (s *S) mutate() {
    s.Lock();
    defer s.Unlock();
    s.index++;
    for i:=0; i< 128; i++ {
        s.values[i]=s.index;
    }
}

func (s S) validate() {
    for i:=0;i<128;i++ {
        if s.values[i]!=s.index {
            log.Fatal("mismatch error")
        }
    }
}

func doit(s *S) {
    for {
        s.mutate()
        s.validate()
    }
}

func main() {
    var s S
    var wg sync.WaitGroup
    wg.Add(1)
    for i:=0;i<64;i++ {
        go doit(&s)
    }
    wg.Wait()
}

In fact, you get a linter warning, because of the copy of the mutex in calling the value method - since it knows it should be a reference.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/B6F948A5-9F2E-4698-85D1-17B862779901%40ix.netcom.com.

[robert engels]

Here is a slightly easier version to see the race between the mutation and the copy for the value method:

package main

import (
    "log"
    "sync"
)

type S struct {

    lock *sync.Mutex

    index int
    values [128]int
}

func (s *S) mutate() {

    s.lock.Lock();
    defer s.lock.Unlock();

    s.index++;
    for i:=0; i< 128; i++ {
        s.values[i]=s.index;
    }
}

func (s S) validate() {
    for i:=0;i<128;i++ {
        if s.values[i]!=s.index {
            log.Fatal("mismatch error")
        }
    }
}

func doit(s *S) {
    for {
        s.mutate()
        s.validate()
    }
}

func main() {
    var s S

    var lock sync.Mutex
    s.lock = &lock

    var wg sync.WaitGroup
    wg.Add(1)
    for i:=0;i<64;i++ {
        go doit(&s)
    }
    wg.Wait()
}

[Axel Wagner]

You are trying to prove something nobody actually doubted. Meanwhile, you seem to be aggressively ignoring what I actually wrote. I find that pretty rude.

[Robert Engels]

You are the rude one - you always have been. You wrote “ I gave reasons why, in my opinion, *not* mixing value and pointer receivers sometimes leads to incorrect code.”

I was pointing out how mixing receiver types can easily lead to unexpected race conditions (ie incorrect code) that are not easily resolvable. I’m sorry if this went over your head. Sheez. 

On Oct 7, 2024, at 11:20 PM, Axel Wagner <axel.wa...@googlemail.com> wrote:



[Robert Engels]

And if you don’t recognize the ass clown rudeness in a statement like “ No offence, but I made an argument. You don't have to agree with the argument and it might be wrong. But to convince me, at least, that argument would need to actually be referenced.” you are a narcissistic ahole. 

On Oct 7, 2024, at 11:20 PM, Axel Wagner <axel.wa...@googlemail.com> wrote:



[Dan Kortschak]

The simple case is due to Dave Cheney[1]. 

package main

import (
"fmt"
"time"
)

type RPC struct {
result int
done chan struct{}
}

func (rpc *RPC) compute() {
time.Sleep(time.Second) // strenuous computation intensifies
rpc.result = 42
close(rpc.done)
}

func (RPC) version() int {
return 1 // never going to need to change this
}

func main() {
rpc := &RPC{done: make(chan struct{})}

go rpc.compute() // kick off computation in the background
version := rpc.version() // grab some other information while we're waiting
<-rpc.done // wait for computation to finish
result := rpc.result

fmt.Printf("RPC computation complete, result: %d, version: %d\n", result, version)
}

[1]https://dave.cheney.net/2015/11/18/wednesday-pop-quiz-spot-the-race

[ren...@ix.netcom.com]

Yep. I wasn’t trying to claim original thinking on this. I had read that long ago. Thanks for referencing. 

[Axel Wagner]

Just to clarify: From what I can tell, I am (in this revival of the thread) the only one on the "mixing receiver kinds is sometimes necessary, so we shouldn't caution against it" side of the table and indeed opened it. As such, I'm already on the back foot. I tried to at least acknowledge the arguments from the other side, even if I don't have much to say about them. I don't believe it's rude to ask for the same from the other side of the table.

I don't expect anyone to be invested in convincing me, as a person. But (again, from what I can tell) I represent a side of the discussion here. And it's not possible to have a discussion where one side is simply ignored.

[Robert Engels]

And when I provided data and reasoning from “the other side of the table” including code samples, you responded with “Meanwhile, you seem to be aggressively ignoring what I actually wrote. I find that pretty rude.”

You need to learn the concept of “in addition to” or “agree, but this is more important” and lose your penchant to immediately rudely escalate the discussion with direct or veiled name calling. 

On Oct 8, 2024, at 1:37 AM, 'Axel Wagner' via golang-nuts <golan...@googlegroups.com> wrote:



You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/_MEf-I49OTo/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfGb7xEu%3Da73xWUBuAGK2T3_R7uA4K5FZYr4vYzkLpTxqg%40mail.gmail.com.

[Ken Lee]

So can I say that, if I'm not writing concurrency code, it's acceptable for me to mix pointer and value receiver? I find that like what @Axel said, I think there's some struct in STD Lib doing this too? case in point, Time struct in "time" package.

[Brian Candler]

On Tuesday 8 October 2024 at 07:37:25 UTC+1 Axel Wagner wrote:

 the only one on the "mixing receiver kinds is sometimes necessary, so we shouldn't caution against it" side of the table

To me this sounds like a false dichotomy. It can be good general good-practice advice to avoid mixing pointer and value receivers (for various reasons including those raised by Robert Engels), and at the same time, in cases where it is necessary to do so, then clearly it has to be done (by definition of "necessary") as long as it's done with care ("caution").

The advice quoted is given under "Some useful guidelines" at https://go.dev/wiki/CodeReviewComments#receiver-type - it's not a hard-and-fast rule - and is aimed "especially to new Go programmers". If you know what you need and why, then that's fine.

As another example: it's general good practice advice to accept interfaces and return concrete types. But for error values, it's necessary to return an interface value. That is an exception to the general rule, but doesn't invalidate it.

[Axel Wagner]

On Wed, 9 Oct 2024 at 10:42, 'Brian Candler' via golang-nuts <golan...@googlegroups.com> wrote:

On Tuesday 8 October 2024 at 07:37:25 UTC+1 Axel Wagner wrote:

 the only one on the "mixing receiver kinds is sometimes necessary, so we shouldn't caution against it" side of the table

To me this sounds like a false dichotomy. It can be good general good-practice advice to avoid mixing pointer and value receivers (for various reasons including those raised by Robert Engels), and at the same time, in cases where it is necessary to do so, then clearly it has to be done (by definition of "necessary") as long as it's done with care ("caution").

Sure. The reason I brought this up in the first place, is that I have not really seen anyone qualifying that advice, anywhere. Not to belabour that point, but before you, no one in this thread acknowledged that there *are* exceptions. And one of the first responses I got was that one of the most popular Go IDEs has a linter enabled by default, that seems to complain about that (I can't confirm that, as I don't use it). So it seemed to me, that a lot of people *do* take it as a pretty hard rule. Not as hard as a compilation error or a vet failure, but still in a way that they think it is generally agreed that it should never be done.

I've just come across this topic a couple of times and even had discussions in code reviews, that linked to the Code Review Comments page. So I felt it's important to qualify it, when it came up here.

 

The advice quoted is given under "Some useful guidelines" at https://go.dev/wiki/CodeReviewComments#receiver-type - it's not a hard-and-fast rule - and is aimed "especially to new Go programmers". If you know what you need and why, then that's fine.

As another example: it's general good practice advice to accept interfaces and return concrete types. But for error values, it's necessary to return an interface value. That is an exception to the general rule, but doesn't invalidate it.

--

You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/f5be82e4-dd89-40c2-9603-f240748e23dcn%40googlegroups.com.

[Axel Wagner]

On Wed, 9 Oct 2024 at 09:54, Ken Lee <ken.lee....@gmail.com> wrote:

So can I say that, if I'm not writing concurrency code, it's acceptable for me to mix pointer and value receiver?

Note that you can't really know, if your users use concurrency or not. And I still maintain, that concurrency is - mostly - a red herring. Yes, calling a method with value receiver is a copy and hence a read, so it is a data race when done concurrently with a write. But there are many possible ways to read or write to a variable.

I still stand by the point that there is no epidemic of data races on time.Time.UnmarshalJSON calls, in practice. So obviously, there *are* ways to mix receiver kinds that does not practically incur this risk of races. I also think, it is a good example to look at how that works: time.Time is *in general* used and passed as a value, so over its normal life, it is never concurrently read/written too - it's mostly read-only. Only in limited circumstances, related to its initial population is it really written to - and those are the calls with pointer receivers. And the same is true for the Enum example I linked: It implements flag.Value and essentially, it is accessed via pointer during flag parsing, but after that used as a value. So make the `Set` method a pointer receiver and anything else a value receiver.

It's a more fuzzy lesson, but I think it's more accurate than talking about concurrency.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/0f008c03-200a-4fa5-8198-3b1f0a227f9cn%40googlegroups.com.

[Ken Lee]

I agree with what Axel said in the below:

```a lot of people *do* take it as a pretty hard rule. Not as hard as a compilation error or a vet failure, but still in a way that they think it is generally agreed that it should never be done. ```

The way I read the https://go.dev/wiki/CodeReviewComments#receiver-type sounds more like a hard rule rather than one with exceptions.

Especially when it's a guide for beginners to Golang, how would they know when are the exceptions when they are new? They might take this as a hard rule and brings up in PR reviews, it would be hard to change one's already established opinion.