CODEWITHSUNDEEP
pointersgostructslice
gempir
gempir

Reputation: 1901

Golang changing values of a struct inside a method of another struct

I have an issue with structs and maybe an issue with pointers, if my guess is correct.

This struct has some fields and a field that holds a slice:

type Bot struct {
    // ...
    connlist []Connection
}

This Connection looks like this:

type Connection struct {
    conn       net.Conn
    messages   int32
    channels   []string
    joins      int32
    connactive bool
}

My problem is changing the value of connactive to true.

Bot has a method that listens to the connection:

func (bot *Bot) ListenToConnection(connection Connection) {
    reader := bufio.NewReader(connection.conn)
    tp := textproto.NewReader(reader)
    for {
        line, err := tp.ReadLine()
        if err != nil {
            log.Printf("Error reading from chat connection: %s", err)
            break // break loop on errors
        }
        if strings.Contains(line, "tmi.twitch.tv 001") {
            connection.activateConn()
        }
        if strings.Contains(line, "PING ") {
            fmt.Fprintf(connection.conn, "PONG tmi.twitch.tv\r\n")
        }
        fmt.Fprintf(bot.inconn, line+"\r\n")
    }
}

And connection.activeConn() is the part that is not working correctly the method looks like this:

func (connection *Connection) activateConn() {
    connection.connactive = true
}

This actually gets executed so it's not an issue of the connection not getting a response or something.

But if I try to loop through it later in a method of Bot, connactive is always false for some reason (which is the default).

for i := 0; i < len(bot.connlist); i++ {
        log.Println(bot.connlist[i].connactive)
}

I think I am working with a copy or so of the original connection and not the changed connection that has connactive = true.

Any ideas? Thanks for the help.

Upvotes: 6

Views: 17409

Answers (2)

icza
icza

Reputation: 417402

Your ListenToConnection() method has one parameter: connection Connection.

When you call this ListenToConnection() method (you didn't post this code), you pass a value of Connection. Everything in Go is passed by value, so a copy will be made of the passed value. Inside ListenToConnection() you operate with this copy. You call its activateConn() method, but that method (which has a pointer receiver) will receive the address of this copy (a local variable).

Solution is simple, change parameter of ListenToConnection() to be a pointer:

func (bot *Bot) ListenToConnection(connection *Connection) {
    // ...
}

Calling it with a value from Bot.connlist:

bot.ListenToConnection(&bot.connlist[0])

A for loop calling it with every elements of conlist:

for i := range bot.connlist {
    bot.ListenToConnection(&bot.conlist[i])
}

Attention! I intentionally used a for ... range which only uses the index and not the value. Using a for ... range with index and value, or just the value, you would observe the same issue (connactive would remain false):

for _, v := range bot.connlist {
    bot.ListenToConnection(&v) // BAD! v is also a copy
}

Because v is also just a copy, passing its address to bot.ListenToConnection(), that would only point to the copy and not the element in the connlist slice.

Upvotes: 11

Piotr Kowalczuk
Piotr Kowalczuk

Reputation: 399

It needs to be slice of pointers to connections. And if this property will be changed concurrently, semaphore is necessary.

type Bot struct {
    // ...
    conns []*Connection
}

func (bot *Bot) ListenToConnection(c *Connection) {
   // code
}

type Connection struct {
    conn         net.Conn
    messages     int32
    channels     []string
    joins        int32
    isActive     bool
    isActiveLock sync.RWMutex
}

func (c *Connection) activateConn() {
    c.isActiveLock.Lock()
    defer c.isActiveLock.Unlock()

    c.isActive = true
}

Upvotes: 1

Related Questions