CODEWITHSUNDEEP
go
Brian Rosner
Brian Rosner

Reputation: 704

Using function to satisfy interface via method

I am trying to write a method that will return a function that can satisfy the json.Marshaler interface. My reasoning is to provide different representations of the struct. Perhaps I am approaching this completely wrong.

func (api *Api) SiteList(c *gin.Context) {
    var sites []db.Site
    if err := api.db.Find(&sites).Error; err != nil {
    }
    var payload []json.Marshaler
    for _, site := range sites {
        payload = append(payload, site.ToApi())
    }
    c.JSON(http.StatusOK, payload)
}

The result I get from this function is the correct number of items in the list, but the same value for each:

[
    {
        "key": "NZ7LCA9HQN3", 
        "name": "autumn-waterfall-1573"
    }, 
    {
        "key": "NZ7LCA9HQN3", 
        "name": "autumn-waterfall-1573"
    }, 
    {
        "key": "NZ7LCA9HQN3", 
        "name": "autumn-waterfall-1573"
    }, 
    {
        "key": "NZ7LCA9HQN3", 
        "name": "autumn-waterfall-1573"
    }, 
    {
        "key": "NZ7LCA9HQN3", 
        "name": "autumn-waterfall-1573"
    }
]

Finally, here is the ToApi implementation:

type EncoderFunc func() ([]byte, error)

func (fn EncoderFunc) MarshalJSON() ([]byte, error) {
    return fn()
}

func (site *Site) ToApi() json.Marshaler {
    return EncoderFunc(func() ([]byte, error) {
        var payload public.Site
        payload.Name = site.Name
        payload.Key = site.Key
        data, err := json.Marshal(payload)
        if err != nil {
            return nil, err
        }
        return data, nil
    })
}

Upvotes: 3

Views: 82

Answers (1)

Ainar-G
Ainar-G

Reputation: 36259

This seems like a classic closure gotcha. There is a related FAQ section.

Basically, site in your for-loop has the same address every time. All your functions are closed over this address. So when you evaluate it after the for-loop, it will repeatedly call MarshalJSON on the value on the same (last) address. You can correct that by creating a new value on every iteration:

for _, site := range sites {
    site := site // Perfectly legal and idiomatic.
    payload = append(payload, site.ToApi())
}

Playground: https://play.golang.org/p/eFujC1hEyD

Another related piece of documentation from Effective Go:

The bug is that in a Go for loop, the loop variable is reused for each iteration, so the req variable is shared across all goroutines. That's not what we want. (...) Another solution is just to create a new variable with the same name, as in this example: 

for req := range queue {
    req := req // Create new instance of req for the goroutine.
    sem <- 1
    go func() {
        process(req)
        <-sem
    }()
}

This section is about goroutines, but apparently this also applies to all closures.

Upvotes: 5

Related Questions