On Mon, Oct 26, 2020 at 12:29 AM Oliver Smith < oliver.sm...@superevilmegacorp.com> wrote:
> This pattern/idiom is very un-golike in that it's an eyesore, and one of > the hardest things I've found to teach people new to trying to read go code. > FWIW, I definitely disagree that this is somehow "un-golike" (non-specific, subjective terms like this have exactly this problem - no one really knows what it means). And I mostly disagree that it's a particularly bad problem - I can see where you are coming from, but at worst, this seems a minor issue, not worth changing the language for. I would also recommend against teaching this to newcomers to the language. Returning a channel in this way has a couple of issues - among others, it doesn't allow the caller to control buffering of the channel. This can be solved (which also, IMO, solves your readability issues) by instead *taking* a channel to write to: func filterPositiveOdds(numbers []int, ch chan<- int) { defer close(channel) for _, value := range numbers { if value > 0 && (value & 1) == 1 { channel <- value } } } func caller() { var numbers []int ch := make(chan int) go filterPositiveOdds(numbers, ch) for n := range ch { } } This has fundamentally the same control-flow, but we give a name to the function, thus making it clearer, what the extra goroutine is for. However, IMO this is still bad form. In general, I would advise exposing channels in APIs. It requires you to specify extra properties, like "what happens if the channel blocks" or "how does the operation get cancelled" in the documentation, without a way to get correctness compiler-checked. In particular, the code (both mine and yours) suffers from exactly these problems - if the channel is not consumed, we leak a goroutine and there is no way to prematurely abort consumption. Getting an error back is even worse. A better way is to provide a simple, synchronous iterator API like bufio.Scanner <https://golang.org/pkg/bufio/#Scanner> does. For example, you could have type IntIterator struct { numbers []int } func FilterPositiveOdds(numbers []int) *IntIterator { return &IntIterator{numbers} } func (it *IntIterator) Next() (n int, ok bool) { for _, n := range it.numbers { it.numbers = it.numbers[1:] if (n > 0 || n & 1 != 0) { return n, true } } return 0, false } In complex cases, concurrency can be hidden behind the iterator API. In simple cases, you could also reduce boilerplate by doing func FilterPositiveOdds(numbers []int) (next func() (n int, ok bool)) { return func() (n int, ok bool) { // same as above, but closing over numbers } } In any case - if you are unhappy with your pattern, there are many alternatives to choose from, within the language as it exists today. It seems hardly worth extra language features, to simplify this IMO rather uncommon construct. > I've toyed with a few alternatives, but I'm having a hard time finding > something I think everyone would like. > > Pattern the first; 'go return` which flips the idiom with syntactic sugar. > 'go return' returns the function and continues the remainder of it in a go > func() { ... } > > ```go > func getPositiveOdds(numbers []int) <-chan int { > channel := make(chan int) > go return channel > > defer close(channel) > for _, value := range numbers { > if value > 0 && (value & 1) == 1 { > channel <- value > } > } > } > ``` > > The second more closely binds to the channel-generator pattern by > extending make(chan) to take a 'go func' parameter: > > ```go > func getPositiveOdds(numbers []int) <-chan int { > generator := make(chan int, go func (channel chan int) { > defer close(channel) > for _, value := range numbers { > if value > 0 && (value & 1) == 1 { > channel <- value > } > } > }) > return generator > } > ``` > > I was tempted by the notion of having this auto-defer-close the channel, > but I think retaining the explicit close is generally better and more > teachable. > > Where it might be tenable would be using syntactic sugar to perhaps better > promote the notion of generators toward 1st class citizens: > > ```go > // 'generator' is syntactic sugar, and instead of a return type you provide > // the make statement to create the required channel type. > generator getPositiveOdds(numbers []int) make(chan int, 1) { > // The variable 'yield' is automatically defined as the channel created > for _, value := range numbers { > if value > 0 && (value & 1) == 1 { > yield <- value > } > } > } > ``` > > which is roughly equivalent to: > > ```golang > func getPositiveOdds(numbers []int]) <-chan int { > generator := func (yield chan int) { > numbers := numbers > for _, value := range numbers { > if value > 0 && (value & 1) == 1 { > yield <- value > } > } > } > > channel := make(chan int, 1) > > go func () { > defer close(channel) > generator(channel) > }() > > return channel > } > ``` > > `gen` instead of `generator` might be ok, I just wanted to be unambiguous. > > -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/golang-nuts/5aa92ac7-1c9b-48af-8389-a6870563b831n%40googlegroups.com > <https://groups.google.com/d/msgid/golang-nuts/5aa92ac7-1c9b-48af-8389-a6870563b831n%40googlegroups.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAEkBMfG7dUX%2BWGes2_-evaGXkzjzx7-cCVxrr0hj9U6e%3D%2BmqHA%40mail.gmail.com.