2

In GoLang, having the following structs and methods, I'm trying to append to a slice that belongs to a struct that is nested in another struct:

/* Tiers agent struct */
type Agent struct {
    Registration string
}

/* Tiers queue struct */
type Queue struct {
    Name string
    Agents []Agent
}

/* Tiers struct */
type Tiers struct {
    Queues []Queue
}

func (Q *Queue) AddAgent (agent_registration string) {

    Q.Agents = append(Q.Agents, Agent{Registration: agent_registration})
}

func (T *Tiers) AddQueue(queue_name string, agent_registration string) {

    var cur_queue *Queue

    found_queue := false

    /* Find queue by queue name */
    for _ , queue := range T.Queues {
        if queue.Name == queue_name {
            found_queue = true
            cur_queue = &queue
        }
    }

    /* If queue not found, create a new one */
    if found_queue == false {
        T.Queues = append(T.Queues, Queue{Name: queue_name})
        cur_queue = &(T.Queues[len(T.Queues)-1])
    }

    /* Add agent to queue (add tier) */
    cur_queue.AddAgent(agent_registration)
}

My business rule is that I'll receive lots of {queue_name, agent_name} pairs and I want to build a structure that groups all agents that shares the same 'queue_name'. So, in the end of day I want something like:

Queue1: ['agent1', 'agent2', 'agent3', ...] Queue2: ['agent4', 'agent5', ...]

I'm modeling this using the structs I've mentioned above. The problem is when I try to add agents to a queue only the last insertion persists. Example:

tiers.AddQueue('suport@default', '1000@default')
tiers.AddQueue('suport@default', '1003@default')
tiers.AddQueue('suport@default', '1001@default')

Output: {[{support@default [{1001@default}]}]}

When what I want is that the output be:

Output: {[{support@default [{1000@default},{1003@default}, {1001@default}]}]}

Pavarine
  • 637
  • 3
  • 15
  • 30

1 Answers1

4

Your problem is that this:

for _ , queue := range T.Queues {

copies values from T.Queues into queue, then you take the address of that copy:

cur_queue = &queue

and end up modifying the copy while leaving the Queue in the T.Queues slice alone.

An easy solution would be to use the for i := range ... form of the loop and take the address of the Queue in the slice rather than the copy:

for i := range T.Queues {
    if T.Queues[i].Name == queue_name {
        found_queue = true
        cur_queue = &T.Queues[i]
    }
}

You could also use a slice of pointers and go back to your

for _ , queue := range T.Queues {

loop with queue already being a pointer.

You might want to break out of the loop early once you've found something. If T.Queues is expected to be large you might want to switch to a map so that you don't have so many linear searches.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • Can I assume that "looking" a key in a map is a O(1) in terms of computational complexity? – Pavarine Jul 26 '18 at 04:03
  • @Pavarine The [spec](https://stackoverflow.com/q/29677670/479863) doesn't guarantee that but you should be safe in assuming that it is a sensible implementation (i.e. amortized O(1)). – mu is too short Jul 26 '18 at 04:23
  • Thank you for the insights, were very clarifyng, I picked the structs solution cause I tought it would be more performatic, even with the linear searchs ( I came from a naive C background), but now I intend to go with the map solution as it looks to be more efficient. Thanks – Pavarine Jul 26 '18 at 04:37