5 ms·

What is wrong with: for { next := getNext() ... } What is the advantage of writing this as: for next := range getNext { .

by rcme 3y ago

What is wrong with: for { next := getNext() ... } What is the advantage of writing this as: for next := range getNext { ... }

bouk 3y ago
The first doesn't do control flow
- rcme 3y ago
  Depends how getNext is defined.
  - alphazard 3y ago
    The only way to do control flow in getNext, consistent with how you partially defined it, is to panic. That means there is some important code wrapping the whole loop body that you left out.
  - zeroxfe 3y ago
    That doesn't make sense. `getNext()` has no ability to break out of the loop.
alphazard 3y ago
In practice the difference would be closer to: getNext := iterableThing.Iterator() for { next, ok := getNext() if !ok { break } ... } vs. for next := range iterableThing.Iterator() { ... } One advantage is that it's slightly shorter, which matters for very common patterns--people complain about `err != nil` after all. Another advantage is there isn't another variable for everyone to name differently. Another advantage is that everyone can't do the control flow slightly differently either.
- tmpz22 3y ago
  > complain about `err != nil` after all Not to nitpick this specifically but as a generic reminder not all complaints are worthy of shifting the trajectory of a massively popular programming language. Balancing "worthy" and "unworthy" changes is really hard both in the community and discussions like this one. I don't envy the teams that have to do it.
  - bombolo 3y ago
    Not forcing to check errors is akin to what you do in C, and even there compilers complain about this.
- JyB 3y ago
  The err!=nil is exactly what you don’t want to mention and the perfect example why you shouldn’t listen to all the close-minded takes some people might have.
- Kamq 3y ago
  I think more idiomatic go for the first case would be: getNext := iterableThing.Iterator() for next, ok := getNext(); ok; next, ok = getNext() { ... } Which, yeah, the range cleans it up a bit, but it's not doing quite as much work as you're implying.
  - davenoul 3y ago
    Don't forget that the important bit in Go is readability of the code. To me the range form is much easier to reason about than this for loop.
    - Kamq 3y ago
      If we're going for readability instead of trying to stick to the case presented above, I'd go with: for i.HasNext() { current := i.GetNext() ... } And just mutate the internal state of the struct. Probably throw it behind some sort of interface like: type Iterator[T any] interface { // feel free to strip out the generic HasNext() bool // if you really only have one type GetNext() T // you do this with } I think this still loses to ranging over a function, but I still think it should be compared to what you can do with the current language as opposed to what was originally posted.
rakoo 3y ago
None of the proposals submit your idea of writing things differently. The article proposes an implementation that is fully doable and usable with current spec and no breaking changes. The point of coroutines is that they are little contexts that serve to be called - many times - sometimes with different parameters - change state - might be interrupted by callers - might interrupt callers themselves All of this can be done by other means. Just like any sorting can be done by copy pasting the same code, but generics make it less tedious. That's the same idea here. Some problems can be implemented as interleaving coroutines, and their definition is simple enough that you want to write it all in the body of some CreateCoroutine() function instead of taking out another struct with 5 almost empty functions. It will not solve all problems, but can more clearly separate business logic and orchestration.
aaronbee 3y ago
Your code is an example of a "pull iterator", and it's not as much of a concern. The harder case to deal with is a "push iterator", which are often much simpler to implement, but less flexible to use. See https://github.com/golang/go/discussions/56413 https://github.com/golang/go/discussions/56413 The OP is about converting a push iterator into a pull iterator efficiently by using coroutines. This provides the best of both worlds, simple iterator implementation and flexible use by its caller.
starttoaster 3y ago
Besides what everyone else said, the obvious advantage is the latter builds in the logic for exiting the loop once getNext has run out of elements in the slice/map. Your former example will need a final step that's like: ... if getNext() == nil { break } ... This isn't a huge boon, and is mostly a matter of style. But I prefer the latter because it's logic that gets handled with the range builtin, so my code can be more about application logic, rather than muddling in breaking out of loop logic.
ben0x539 3y ago
Well, what was wrong with: for { next := <-channel ... }
- Mawr 3y ago
  Horrible overhead. If the loop does something simple, like summing integers, 99% of time will be spent switching between goroutines. From TFA: "Because scheduling is explicit (without any preemption) and done entirely without the operating system, a coroutine switch takes at most around ten nanoseconds, usually even less. Startup and teardown is also much cheaper than threads." "For this taxonomy, Go's goroutines are cheap threads: a goroutine switch is closer to a few hundred nanoseconds" Also check out https://news.ycombinator.com/item?id=29510751 https://news.ycombinator.com/item?id=29510751 and https://ewencp.org/blog/golang-iterators/index.html https://ewencp.org/blog/golang-iterators/index.html: "Finally, the most natural channel based implementation is… slow. By a factor of nearly 50x. Buffering does help though, reducing that to a factor of 25x."
  - unscaled 3y ago
    And even if it's a toy program and you don't care about performance, it's not as simple as just: for { next := <-channel ... } You have to set up the channel and the goroutine that feeds it, you need to safely close the channel when the iteration is done (but not before, unless you like panics), you need to deal with panics inside the goroutine and possibly support cancellation if the iteration breaks early (unless you love memory leaks). If you try to implement this pattern by hand, you are all too likely to make fatal mistake, and this is doubly true in the hands of an inexperienced programmer. I appreciate the fact that Russ wrote this long post, gradually implementing `coro.New()` and improving its functionality and safety — and only in the very end, we get a short paragraph about performance. Good performance is important to make this feature attractive to use, but if the feature is clunky and error-prone, it wouldn't be worth much, even with great performance.
  - ben0x539 3y ago
    I'm responding to a comment asking for a justification for sugar for function call based iteration. My comment is an attempt to draw a parallel to the need for sugar for channel based iteration. I'm not trying to suggest channel based iteration as an alternative to coroutines.

by rcme 3y ago

What is wrong with: for { next := getNext() ... } What is the advantage of writing this as: for next := range getNext { ... }

bouk 3y ago

The first doesn't do control flow

rcme 3y ago
Depends how getNext is defined.
- alphazard 3y ago
  The only way to do control flow in getNext, consistent with how you partially defined it, is to panic. That means there is some important code wrapping the whole loop body that you left out.
- zeroxfe 3y ago
  That doesn't make sense. `getNext()` has no ability to break out of the loop.

alphazard 3y ago

In practice the difference would be closer to: getNext := iterableThing.Iterator() for { next, ok := getNext() if !ok { break } ... } vs. for next := range iterableThing.Iterator() { ... } One advantage is that it's slightly shorter, which matters for very common patterns--people complain about `err != nil` after all. Another advantage is there isn't another variable for everyone to name differently. Another advantage is that everyone can't do the control flow slightly differently either.

tmpz22 3y ago
> complain about `err != nil` after all Not to nitpick this specifically but as a generic reminder not all complaints are worthy of shifting the trajectory of a massively popular programming language. Balancing "worthy" and "unworthy" changes is really hard both in the community and discussions like this one. I don't envy the teams that have to do it.
- bombolo 3y ago
  Not forcing to check errors is akin to what you do in C, and even there compilers complain about this.
JyB 3y ago
The err!=nil is exactly what you don’t want to mention and the perfect example why you shouldn’t listen to all the close-minded takes some people might have.
Kamq 3y ago
I think more idiomatic go for the first case would be: getNext := iterableThing.Iterator() for next, ok := getNext(); ok; next, ok = getNext() { ... } Which, yeah, the range cleans it up a bit, but it's not doing quite as much work as you're implying.
- davenoul 3y ago
  Don't forget that the important bit in Go is readability of the code. To me the range form is much easier to reason about than this for loop.
  - Kamq 3y ago
    If we're going for readability instead of trying to stick to the case presented above, I'd go with: for i.HasNext() { current := i.GetNext() ... } And just mutate the internal state of the struct. Probably throw it behind some sort of interface like: type Iterator[T any] interface { // feel free to strip out the generic HasNext() bool // if you really only have one type GetNext() T // you do this with } I think this still loses to ranging over a function, but I still think it should be compared to what you can do with the current language as opposed to what was originally posted.

rakoo 3y ago

None of the proposals submit your idea of writing things differently. The article proposes an implementation that is fully doable and usable with current spec and no breaking changes. The point of coroutines is that they are little contexts that serve to be called - many times - sometimes with different parameters - change state - might be interrupted by callers - might interrupt callers themselves All of this can be done by other means. Just like any sorting can be done by copy pasting the same code, but generics make it less tedious. That's the same idea here. Some problems can be implemented as interleaving coroutines, and their definition is simple enough that you want to write it all in the body of some CreateCoroutine() function instead of taking out another struct with 5 almost empty functions. It will not solve all problems, but can more clearly separate business logic and orchestration.

aaronbee 3y ago

Your code is an example of a "pull iterator", and it's not as much of a concern. The harder case to deal with is a "push iterator", which are often much simpler to implement, but less flexible to use. See https://github.com/golang/go/discussions/56413 https://github.com/golang/go/discussions/56413 The OP is about converting a push iterator into a pull iterator efficiently by using coroutines. This provides the best of both worlds, simple iterator implementation and flexible use by its caller.

starttoaster 3y ago

Besides what everyone else said, the obvious advantage is the latter builds in the logic for exiting the loop once getNext has run out of elements in the slice/map. Your former example will need a final step that's like: ... if getNext() == nil { break } ... This isn't a huge boon, and is mostly a matter of style. But I prefer the latter because it's logic that gets handled with the range builtin, so my code can be more about application logic, rather than muddling in breaking out of loop logic.

ben0x539 3y ago

Well, what was wrong with: for { next := <-channel ... }

Mawr 3y ago
Horrible overhead. If the loop does something simple, like summing integers, 99% of time will be spent switching between goroutines. From TFA: "Because scheduling is explicit (without any preemption) and done entirely without the operating system, a coroutine switch takes at most around ten nanoseconds, usually even less. Startup and teardown is also much cheaper than threads." "For this taxonomy, Go's goroutines are cheap threads: a goroutine switch is closer to a few hundred nanoseconds" Also check out https://news.ycombinator.com/item?id=29510751 https://news.ycombinator.com/item?id=29510751 and https://ewencp.org/blog/golang-iterators/index.html https://ewencp.org/blog/golang-iterators/index.html: "Finally, the most natural channel based implementation is… slow. By a factor of nearly 50x. Buffering does help though, reducing that to a factor of 25x."
- unscaled 3y ago
  And even if it's a toy program and you don't care about performance, it's not as simple as just: for { next := <-channel ... } You have to set up the channel and the goroutine that feeds it, you need to safely close the channel when the iteration is done (but not before, unless you like panics), you need to deal with panics inside the goroutine and possibly support cancellation if the iteration breaks early (unless you love memory leaks). If you try to implement this pattern by hand, you are all too likely to make fatal mistake, and this is doubly true in the hands of an inexperienced programmer. I appreciate the fact that Russ wrote this long post, gradually implementing `coro.New()` and improving its functionality and safety — and only in the very end, we get a short paragraph about performance. Good performance is important to make this feature attractive to use, but if the feature is clunky and error-prone, it wouldn't be worth much, even with great performance.
- ben0x539 3y ago
  I'm responding to a comment asking for a justification for sugar for function call based iteration. My comment is an attempt to draw a parallel to the need for sugar for channel based iteration. I'm not trying to suggest channel based iteration as an alternative to coroutines.