CODEWITHSUNDEEP
go
dmc94
dmc94

Reputation: 536

How can I dynamically populate a struct?

I want to dynamically populate my internal struct, for an atomic insert. I am new to go so pointers and referencing them is something that I am still learning. I can not figure out why this for each loop is putting the same fields in twice. I tried removing the '&' then I get a cannot use type as *type error, I checked to make sure my loop was hitting every object in the tradeArray, and it is. It looks like it is overwriting the object before it with the last one it loops over. How can I fix this?

   func createTrade(w http.ResponseWriter, r *http.Request) {
        w.Header().Set("Content-Type", "application/json")

        var tradeArray []Trade
    
    
        if err := json.NewDecoder(r.Body).Decode(&tradeArray); err != nil {
            e := Error{Message: "Bad Request - Improper Types Passed"}
            w.WriteHeader(http.StatusBadRequest)
            _ = json.NewEncoder(w).Encode(e)
            return
        }
    
    
        for _, trade := range tradeArray {
            internal := InternalTrade{
                Id: strconv.Itoa(rand.Intn(1000000)),
                Trade: &trade,
            }
            submit := TradeSubmitted{
                TradeId:       internal.Id,
                ClientTradeId: trade.ClientTradeId ,
            }
            submitArray = append(submitArray, submit)
            trades = append(trades, internal)
        }
    
    
        if err := json.NewEncoder(w).Encode(submitArray); err != nil {
            e := Error{Message:"Internal Server Error"}
            w.WriteHeader(http.StatusInternalServerError)
            _ = json.NewEncoder(w).Encode(e)
            return
        }
    }

edit: I was able to fix my problem by creating a new variable to hold the trade and referencing that variable in the struct creation. I am not sure how this is different that what I was doing above with just referencing the "trade" if someone could explain that I would greatly appreciate it.

    for _, trade := range tradeArray {
    p := trade
    internal := InternalTrade{
        Id: strconv.Itoa(rand.Intn(1000000)),
        Trade: &p,
    }
    submit := TradeSubmitted{
        TradeId:       internal.Id,
        ClientTradeId: trade.ClientTradeId ,
    }
    submitArray = append(submitArray, submit)
    trades = append(trades, internal)
}

Upvotes: 0

Views: 63

Answers (1)

torek
torek

Reputation: 487695

Let's look at just these parts:

    var tradeArray []Trade

    // code that fills in `tradeArray` -- correct, and omitted here

    for _, trade := range tradeArray {
        internal := InternalTrade{
            Id: strconv.Itoa(rand.Intn(1000000)),
            Trade: &trade,
        }
        submit := TradeSubmitted{
            TradeId:       internal.Id,
            ClientTradeId: trade.ClientTradeId ,
        }
        submitArray = append(submitArray, submit)
        trades = append(trades, internal)
    }

This for loop, as you have seen, doesn't work the way you want. Here's a variant of it that's kind of similar, except that the variable trade has scope that extends beyond the for loop:

    var trade Trade
    for i := range tradeArray {
        trade = tradeArray[i]
        internal := InternalTrade{
            Id: strconv.Itoa(rand.Intn(1000000)),
            Trade: &trade,
        }
        // do correct stuff with `internal`
    }

Note that each internal object points to a single, shared trade variable, whose value gets overwritten on each trip through the loop. The result is that they all point to the one from the last trip through the loop.

Your fix is itself OK: each trip through the loop, you make a new (different) p variable, and use &p, so that each internal.Trade has a different pointer to a different copy. You could also just do trade := trade inside the loop, to create a new unique trade variable. However, in this particular case, it may make the most sense to rewrite the loop this way:

    for i := range tradeArray {
        internal := InternalTrade{
            Id: strconv.Itoa(rand.Intn(1000000)),
            Trade: &tradeArray[i],
        }
        // do correct stuff with `internal`
    }

That is, you already have len(tradeArray) different Trade objects: the slice header tradeArray gives you access to each tradeArray[i] instance, stored in the underlying array. You can just point to those directly.

There are various advantages and disadvantages to this approach. The big advantage is that you don't re-copy each trade at all: you just use the ones from the array that the slice header covers, that was allocated inside the json Decode function somewhere. The big disadvantage is that this underlying array cannot be garbage-collected as long as you retain any pointer to any of its elements. That disadvantage may have no cost at all, depending on the structure of the remaining code, but if it is a disadvantage, consider declaring tradeArray as:

var tradeArray []*Trade

so that the json Decode function allocates each one separately, and you can point to them one at a time without forcing the retention of the entire collection.

Upvotes: 2

Related Questions