Behavior of unmarshalling JSON into a reused slice

Getting Help
1

Hello all,

I have code that calls an internal API. That API will return a list of unique consumer IDs in ascending order. There are no duplicates in the results returned from the API. I can limit the number of results returned, and then page through subsequent calls to the API. This lets me build up a slice of consumer IDs. I’ve noticed that I sometimes wind up with duplicate consumer IDs in my results, and I don’t understand why.

// Members holds the response from the internal API
type MemberResponse struct {
    Request   string
    Total     int
    Members   []int
}

// This code can result in duplicates when paging
func GetMembers() []int {
    var resp MemberResponse
    // Make call to API which will return the response
    callApi(&resp)
    // Store first response
    members := resp.Members

    // Page through more responses
    done := false
    for !done {
        callApi(&resp)
        members = append(members, resp.Members...)
        // paging logic to set done...
    }
    return members
}

// If I "reset" the response's slice before subsequent calls, I do not get duplicates
func GetMembers() []int {
    var resp MemberResponse
    // Make call to API which will return the response
    callApi(&resp)
    // Store first response
    members := resp.Members

    // Page through more responses
    done := false
    for !done {
        resp.Members = nil  // <--- change
        callApi(&resp)
        members = append(members, resp.Members...)
        // paging logic to set done...
    }
    return members
}

I believe after the first API call, members is a new slice that shares the underlying array of resp.Members. My assumption is that the subsequent API call then modifies that same underlying array and then I append the modified array to itself, which then winds up overlapping some of the consumer IDs causing duplicates in members.

Am I thinking about this in the right way? That setting resp.Members = nil or, alternatively using:

  var members []int
  members = append(members, resp.Members)

To make a copy of the original results is truly the right thing to do? The more I think about it, the more it appear that my first version of that function is just a terrible idea.

2

Wild guess: have you tried using the sync package?

var mtx sync.Mutex

mtx.Lock()
members = append(members, resp.Members...)
mtx.Unlock()
3

I haven’t, but there aren’t multiple go routines accessing this code, so I don’t believe it is a synchronization issue.

4

Maybe callApi(&resp) sometimes returns fewer member ID’s than the previous calls did, without shortening the resp.Members slice accordingly. In this case, callApi would leave some of the previous values at the end of resp.Members untouched.

5

Yes. Otherwise the members slice will indeed reference the same slice you are unmarshalling into, and at least the second call to the API will overwrite things there.

Creating an empty result slice, unmarshalling into something else, and appending that something else to the result slice after each call is correct. The “something else” can of course be reused for every API call, as you do.

6

Hey @merickson, you’ve already had some good responses here so far, but I’ll just chuck in my 2 cents.

Personally I would recommend changing the function signature of callApi to return a []int rather than taking in a pointer to a MembersResponse.

Here is an example of what I mean:

func callApi() []int {
	// members := ...
	// return members
}

and then in GetMembers:

func GetMembers() []int {
	var resp MemberResponse
	resp.Members = callApi() // initial response.

	done := false
	for !done {
		moreMembers := callApi()
		resp.Members = append(resp.Members, moreMembers...)
	}

	return resp.Members
}

However, I would even recommend that you simply do this for GetMembers if you are only using the members field from the var resp MemberResponse in the function:

func GetMembers() []int {
	members := callApi() // initial response, no need for whole MemberResponse object.

	done := false
	for !done {
		moreMembers := callApi()
		members = append(members, moreMembers...)
	}

	return members
}
1 Like
7

Thanks for the responses everyone. I do appreciate it.

@radovskyb, I get what you mean. The callAPI() is actually a bit more generic than I’ve represented here. Its arguments are really a struct with a set of credentials, a URL, and a pointer to an interface that knows how to un-marshal things. That way we use the same code to call all of the various APIs in our application.

8

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.