CODEWITHSUNDEEP
go
alecbz
alecbz

Reputation: 6488

Better go-idiomatic way of writing this code?

Playing around with go, I threw together this code:

package main

import "fmt"

const N = 10

func main() {
    ch := make(chan int, N)
    done := make(chan bool)

    for i := 0; i < N; i++ {
        go (func(n int, ch chan int, done chan bool) {
            for i := 0; i < N; i++ {
                ch <- n*N + i
            }
            done <- true
        })(i, ch, done)
    }

    numDone := 0
    for numDone < N {
        select {
        case i := <-ch:
            fmt.Println(i)
        case <-done:
            numDone++
        }
    }

    for {
        select {
        case i := <-ch:
            fmt.Println(i)
        default:
            return
        }
    }
}

Basically I have N channels doing some work and reporting it on the same channel -- I want to know when all the channels are done. So I have this other done channel that each worker goroutine sends a message on (message doesn't matter), and this causes main to count that thread as done. When the count gets to N, we're actually done.

Is this "good" go? Is there a more go-idiomatic way of doing this?

edit: To clarify a bit, I'm doubtful because the done channel seems to be doing a job that channel closing seems to be for, but of course I can't actually close the channel in any goroutine because all the routines share the same channel. So I'm using done to simulate a channel that does some kind of "buffered closing".

edit2: Original code wasn't really working since sometimes the done signal from a routine was read before the int it just put on ch. Needs a "cleanup" loop.

Upvotes: 4

Views: 511

Answers (6)

Bill DeRose
Bill DeRose

Reputation: 2450

I was dealing with the same issue in some code of mine and found this to be a more than adequate solution.

The answer provides Go's idiom for handling multiple goroutines all sending across a single channel.

Upvotes: 0

nemo
nemo

Reputation: 57609

If you're iterating over values generated from goroutines, you can iterate directly over the communication channel:

for value := range ch {
   println(value)
}

The only thing necessary for this is, that the channel ch is closed later on, or else the loop would wait for new values forever.

This would effectively replace your for numDone < N when used in combination with sync.WaitGroup.

Upvotes: 0

Nick Craig-Wood
Nick Craig-Wood

Reputation: 54081

Here is an idiomatic use of sync.WaitGroup for you to study

(playground link)

package main

import (
    "fmt"
    "sync"
)

const N = 10

func main() {
    ch := make(chan int, N)
    var wg sync.WaitGroup
    for i := 0; i < N; i++ {
        wg.Add(1)
        go func(n int) {
            defer wg.Done()
            for i := 0; i < N; i++ {
                ch <- n*N + i
            }
        }(i)
    }
    go func() {
        wg.Wait()
        close(ch)
    }()
    for i := range ch {
        fmt.Println(i)
    }
}

Note the use of closures in the two go routine definitions and note the second go statement to wait for all the routines to finish, then close the channel, so range can be used.

Upvotes: 10

zzzz
zzzz

Reputation: 91193

In the first approximation the code seems more or less okay to me.

Wrt the details, the 'ch' should be buffered. Also the 'done' channel goroutine "accounting" might be possibly replaced with sync.WaitGroup.

Upvotes: 1

cthom06
cthom06

Reputation: 9635

looks like you want a sync.WaitGroup (http://golang.org/pkg/sync/#WaitGroup)

Upvotes: 2

amattn
amattn

Reputation: 10065

Just use a WaitGroup! They are the built-in primitive that essentially let you wait for stuff in different goroutines to finish up.

http://golang.org/pkg/sync/#WaitGroup

As for your doubts, The way to thing about is that being done by closing a channel (done permanently) and being done with work (temporarily) are different.

Upvotes: 1

Related Questions