Cannot assign variable to anonymous func in for loop

Question

I'm studying go-lang by developing a task schedular. The cron library I use accepts a cron expression and a func as parameters to add a scheduler.

c.AddFunc("0 30 * * * *", func() { fmt.Println("Every hour on the half hour") })

The schedular I am developing schedules jobs according to a yaml file. So I iterate the jobs to add schedulars like this:

type Job struct {
    Name        string
    Interval    string

}

func DistributeJob(job Job) {
    log.Println("running", job, job.Interval)
}

func main() {
    //load config from yaml
    c := cron.New()

    for _, job := range config.Jobs {
        c.AddFunc("@every "+job.Interval, func() {
            DistributeJob(job)
        })
        log.Println("Job " + job.Name + " has been scheduled!")
    }

    c.Start()
    select {}
}

All jobs are scheduled on their intervals but it turns out that they're printing the last job's description. For example, if I schedule two jobs, the first interval on 3min and the latter interval on 1min. The console prints:

12:01: Running latter 1min
12:02: Running latter 1min
12:03: Running latter 1min
12:03: Running latter 1min//this one should be the first job

I think the problem is at

    func() {
        DistributeJob(job)
    })

It seems it only takes the last job but I can't figure out why. I tried using

    c.AddFunc("@every "+job.Interval, func(job JobType) {
        DistributeJob(job)
    }(job))

but it fails due to cannot used as value

Evan · Accepted Answer · 2015-03-10 01:55:38Z

9

While you are not using goroutines, the mistake you are making is almost identical to the one described here: https://github.com/golang/go/wiki/CommonMistakes#using-closures-with-goroutines

To quote:

The val variable in the above loop is actually a single variable that takes on the value of each slice element. Because the closures are all only bound to that one variable, there is a very good chance that when you run this code you will see the last element printed for every iteration instead of each value in sequence, because the goroutines will probably not begin executing until after the loop.

So your attempted fix (to pass it as a parameter to your function) would in principle fix the problem, however you cannot pass a function with parameters to the cron library - a function with parameters is a different type from one without (in addition to which by adding the () you are actually calling the function in advance and trying to pass its return value).

The simplest fix is to create a new variable for every iteration of the loop and avoid the whole problem, like so:

for _, job := range config.Jobs {
    realJob := job // a new variable each time through the loop
    c.AddFunc("@every "+realJob.Interval, func() {
        DistributeJob(realJob)
    })
    log.Println("Job " + realJob.Name + " has been scheduled!")
}

answered Mar 10, 2015 at 1:55

Evan

6,5731 gold badge31 silver badges30 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Dave C Over a year ago

You can even do: job := job to get a new variable with the same name (that shadows the old one). It's legal and works and is even mentioned in Effective Go (search for req := req).

Collectives™ on Stack Overflow

Cannot assign variable to anonymous func in for loop

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related