2

I wanted to try the FizzBuzz test (Why can't programmers program), and used Go. It's basically looping from 1 to 100, and printing "Fizz" when the loop counter is divisible by 3, "Buzz" when divisible by 5, "FizzBuzz" when divisible by both and else just print the number.

After doing it iteratively and recursively, I wanted to do it concurrently (or by using channels). I came up with the following code, which worked to my surprise:

func fizzbuzzconc() {
    // Channels for communication
    fizzchan := make(chan int)
    buzzchan := make(chan int)
    fizzbuzzchan := make(chan int)
    nonechan := make(chan int)

    // Start go routine to calculate fizzbuzz challenge
    go func() {
        for i := 1; i <= 100; i++ {
            if i % 3 == 0 && i % 5 == 0 {
                fizzbuzzchan <- i
            } else if i % 3 == 0 {
                fizzchan <- i
            } else if i % 5 == 0 {
                buzzchan <- i
            } else {
                nonechan <- i
            }
        }
    }()

    // When or how does this for loop end?
    for {
        select {
        case i := <-fizzchan:
            fmt.Println(i, "Fizz")
        case i := <-buzzchan:
            fmt.Println(i, "Buzz")
        case i := <-fizzbuzzchan:
            fmt.Println(i, "FizzBuzz")
        case i  := <-nonechan:
            fmt.Println(i, i)
        }
    }
}

I can't understand how and why the for loop stops. There is no break condition, or a return statement. Why does it eventually finish running?

Kiril
  • 6,009
  • 13
  • 57
  • 77
  • Add a `default` to your select. Any change? – bishop Sep 03 '14 at 16:42
  • When I add a default the execution gets much slower, and it doesn't exit. Now I have the expected behaviour, thanks. – Kiril Sep 03 '14 at 16:50
  • 1
    Yep. The reason that happens is that all your `case` enter a system call via `Println`. Go checks channel state during system calls and sees, when the last one empties, that all the channels are done. At that point, it's deadlock detector kicks in and kills the loop. When you add an empty `default`, there's a branch that doesn't go through system calls and go just spins through it. The smallest change you can make to get your code to exit cleanly is to add a wait group channel close in your default case, as seen in this answer: http://stackoverflow.com/a/21819794/2908724 – bishop Sep 03 '14 at 17:05

3 Answers3

1

It doesn't really work well.

What happens, after a time, is that there's an attrition due to the remaining go-routine waiting for a channel where no goroutine pushes. So what you have is a dead-lock (which is a fatal error ending the program), not a clean ending.

In summary it "works" because the go engine is smart enough to detect the dead lock.

Ralph Caraveo
  • 10,025
  • 7
  • 40
  • 52
Denys Séguret
  • 372,613
  • 87
  • 782
  • 758
  • `which is a fata error ending the program` but it ends without errors. Shouldn't it panic? – Kiril Sep 03 '14 at 16:39
  • You are right, there is an error. On my machine (go version go1.3 windows/amd64) there's no error. What would it take to make it work, or what would be an optimal solution to the problem? Just add another channel, that sends a value once the go routine is done? – Kiril Sep 03 '14 at 16:45
  • 3
    @Kiril Or use a sync.WaitGroup – OneOfOne Sep 03 '14 at 17:05
0

@dystroy has answered your question very well, however here is how you might fix your code.

One way of exiting tidily is using a quit channel which we signal by closing it. (Signalling by closing a channel is useful because more than one go routine can be listening on it at once).

There are other ways of doing this though - if you only have one output channel then you range over it to read the result and close it when you finished. You could easily re-write this to work that way.

You can use a sync.Waitgroup to make sure the go routine has finished also.

Playground

func main() {
    // Channels for communication
    fizzchan := make(chan int)
    buzzchan := make(chan int)
    fizzbuzzchan := make(chan int)
    nonechan := make(chan int)
    quit := make(chan struct{})

    // Start go routine to calculate fizzbuzz challenge
    go func() {
        for i := 1; i <= 100; i++ {
            if i%3 == 0 && i%5 == 0 {
                fizzbuzzchan <- i
            } else if i%3 == 0 {
                fizzchan <- i
            } else if i%5 == 0 {
                buzzchan <- i
            } else {
                nonechan <- i
            }
        }
        close(quit)
    }()

    // When or how does this for loop end?
OUTER:
    for {
        select {
        case i := <-fizzchan:
            fmt.Println(i, "Fizz")
        case i := <-buzzchan:
            fmt.Println(i, "Buzz")
        case i := <-fizzbuzzchan:
            fmt.Println(i, "FizzBuzz")
        case i := <-nonechan:
            fmt.Println(i, i)
        case <-quit:
            break OUTER
        }
    }
    fmt.Println("All done")
}
Nick Craig-Wood
  • 52,955
  • 12
  • 126
  • 132
0

@OneOfOne mentioned the sync.WaitGroup method which I think falls most in line with how you would do this in Go. Considering goroutines are very cheap and the problem can be solved in parallel, we can create a go routine for each input and send the results on a buffered channel.

//Size to check
size := 100

results := make(chan Result, size)

// Create the WaitGroup and set the latch size.
var wg sync.WaitGroup
wg.Add(size)

// Create a goroutine for each parallel operation
for i := 1; i <= size; i++ {
    i := i  //bind value into closure
    go func() {
        results <- fizzbuzz(i)
        wg.Done()  //release a latch
    }()
}

//wait for all the goroutines to finish.
wg.Wait()

//close the channel so we can exit the below for loop.
close(results)

//Range over the results and exit once the channel has closed.
for x := range results {
    fmt.Printf("i: %d result: %s\n", x.Nr, x.Val)
}

Playground code: http://play.golang.org/p/80UafMax7M

Nano
  • 51
  • 2