Hacker News new | ask | show | jobs
by knorker 3105 days ago
#4 can also be fixed with:

    for i := 0; i < 10; i++ {
      i := i  // Creates a *new* `i`
      defer func() { … something with i … }
      go func() { … something with i … }
    }
3 comments

It would be nice if Go made a new i for each iteration of the loop.

Lua 5.0 made just one loop variable per loop, like Go apparently does. When I first ran into this behavior, I thought that, although it wasn't what I expected, it made at least as much sense as what I was expecting. Since then, every single time I've been in a situation where the two ways of doing it gave different results, I've wanted the "separate variable for each iteration" behavior, so I was glad when Lua's creators changed loop variables to work like this in Lua 5.1.

(Before this change I would just do the Lua equivalent of your "Creates a new `i`" line.)

C# also switched to a new i for each iteration.
Crikey! You wouldn't wanna allocate closures in a loop anyway, would you? Even if the compiler is smart about it. Doesn't read so well either.. what's wrong with adding a simple argument to your anonymous-func allocated outside of a loop? It's readable, and there are no 'gotchas'. The args are evaluated at the point of defer/go, not func execution. Simples.

Shadowing bites you sooner or later if it becomes a habit and gains you almost nothing in real terms. I keep running into subtle nasties in coworkers' commits from quick'n'dirty-that-stayed "convenience" shadowings. Need I say `err`.. of course linters help, but aren't a given in a "bring-your-own chosen dev-env" team. =)

> Crikey! You wouldn't wanna allocate closures in a loop anyway, would you?

Very often yes I do.

Go encourages synchronous APIs, making it up to the caller to add concurrency. This is great, in my opinion. E.g. this is a common pattern:

  var wg sync.WaitGroup
  ch := make(chan int, len(items))
  for _, item := range items {
    item := item
    wg.Add(1)
    go func() {
      defer wg.Done()
      ch <- doSomething(item)
    }()
  }
  wg.Wait()
  close(ch)
  for res := range ch {
    …
  }
Similar patterns with defer, although yes I'd say less often in a loop. Though in order to be "exception safe" (panic safe) I often do:

  foo, err := openFileOrSomething()
  defer foo.Close()
  [… do something …]
  err := foo.Close()
So that even if "do something" throws exception… err… panics… the file gets closed. And double-closing is safe. That's not a closure though, in this example. So maybe not so good.
None of your explanations/examples counter the fact that you could also declare your anonymous-func just above your loop in a local, ie. `doit:= func(item...` and then simply `go doit(item)` in the loop. You get: a leaner terser loop to later have to read through, no iteration-scope "gotcha", no need to elaborately spell out explicit shadowings for what are naturally semantically really func "args" already/anyway, at worst identical cost (or better)..

But well, guess it comes down to subjective stylistic preferences here =)

> So that even if "do something" throws exception… err… panics… the file gets closed.

Your defer as placed in your above example is already scheduled to run always, even on a later panic. (After all, how else could one `recover` from a `panic` if it wasn't for `defer`?) I don't see the point of the double-closing at all here..

Yeah, I could declare it above. But it's subjectively harder to read, having to jump around. Like you say: subjective.

> Your defer as placed in your above example is already scheduled to run always

Yes, brainfart. Sorry. This is from a completely separate recommendation to defer a close (dropping error return), but also manually close so that closing errors can be surfaced. Matters for e.g. writing files (not so much reading), especially when you don't flush manually.

This is too sneaky and seems like poor form.

I'm curious what good use-case there is for this, that the language bothered to support and allow this to even compile.

Duplicate variable name declaration + referencing in the same scope is unintuitive at best, and just seems wrong.

Shadowing can be useful if you want to make sure the "old" variable is never referenced again in the same scope.

The reason why this idiom feels "weird" is that the loop construct of the language really ought to automatically make a fresh instance of the loop counter for each trip through the loop.

I don't feel that it's in more poor form than the solution suggested in the article:

    for i := 0; i < 3; i++ {
      defer func(i int) {
       fmt.Println(i)
      }(i)
    }
This shadows `i` pretty much the same amount as what I wrote. If `i` gets a different name inside the lambda, then it'd also be worse because then you could accidentally use `i` still.
I personally prefer the version presented in the article, but your version has at least one nice feature in that you can cleanly specify at the top of the loop body which variables are being shadowed.