Editing a struct list variable using pointer not working as expected in go

Question

I have a struct that looks like

type Request struct {
    Name string `json:"name"`
    Parameters []Parameter `json:"parameters"`
}

and

type Parameter struct {
    Attached bool `json:"attached"`
    Script string `json:"script"`
}

Now, I have unmarshalled the json into the struct, and the Script variable has an http location "http://localhost/helloworld.sh". What I am trying to do is, to change the struct variable Parameter.Script from http://localhost/helloworld.sh with the actual content of the script, which is a plain ascii shell script. I wrote a method for the inner struct like

func (p *Parameter) SetScript(script string)  {
    p.Script = script
}

using pointer to Parameter,

and in the GetScript function, try to call that method after getting the response body.

func GetScript(params *Request)  {
  for _, i := range params.Parameters {
    switch i.Attached {
    case false:
        client := new(http.Client)
        req, _ := http.NewRequest("GET", i.Script, nil)
        resp, _ := client.Do(req)
        defer resp.Body.Close()
        reader, _ := ioutil.ReadAll(resp.Body)
        i.SetScript(string(reader))
    }
  }
}

However, when I print the struct after calling this function, it has not modified the variables, and prints the http://localhost/helloworld.sh only. I am able to get the response body, which is the actual content of the script, but I am not able to replace the struct variable from within the GetScript function. Could someone please point out the right way to do this?

Thank you.

Possible duplicate of [Assigned pointer field becomes ](http://stackoverflow.com/questions/34760950/assigned-pointer-field-becomes-nil). Using `for _, i := range` the loop variable is a copy of the slice element, and you modify the copy, not the element in the slice. Use `for i:= range` and refer to `params.Parameters[i]`. — icza, Jun 15 '16 at 13:04
Thank you @icza for pointing to a similar question, but I tried that as well, as `script := string(reader); &i.Script = &script`, which threw an error saying `cannot assign to &i.Script` and nothing else. — nohup, Jun 15 '16 at 13:09
Thank you @icza, that worked. If you can post it as an answer, I can accept and mark it as solved. — nohup, Jun 15 '16 at 13:21

icza · Accepted Answer · 2016-06-15T14:18:29.967

The problem is that you are using a for _, i := range loop, and you modify the loop variable inside the loop:

for _, i := range params.Parameters {
    switch i.Attached {
    case false:
        // ...
        i.SetScript(string(reader))
    }
}

The loop variable i is a copy of the slice elements you range over. So if you do any modification to it, you will only modify the copy and not the element in the slice. (Note that it's true that the SetScript() method has a pointer receiver, but it will receive the address of the copy, so it can and will modify only the copy.)

One workaround is use an index-only range, and refer to the slice element using indexing (replace all occurrence of i with params.Parameters[i]):

for i := range params.Parameters {
    switch params.Parameters[i].Attached {
    case false:
        // ...
        params.Parameters[i].SetScript(string(reader))
    }
}

You can simplify the above code by assigning the slice to a local variable (this will copy only the slice header not its elements, and it will refer to the same underlying array), and use if statement instead of that ugly switch:

p := params.Parameters
for i := range p {
    if !p[i].Attached {
        // ...
        p[i].SetScript(string(reader))
    }
}

An alternative simplification / improvement is to take the address of the index expression, and use that (so you can omit repeating it multiple times):

for i := range params.Parameters {
    p := &params.Parameters[i]
    if !p.Attached {
        // ...
        p.SetScript(string(reader))
    }
}

Editing a struct list variable using pointer not working as expected in go

1 Answers1