Is it OK to use channels in this way?

Or is this some sort of channel/pipe crime?!

So I was tackling this problem on Todd McCleod’s Udemy course. The exercise is to convert a program which uses two go routines (which add to a number) and an atomic, to using channels.

I spent a while trying to figure this out, and took the ‘share memory by communicating’ proverb fairly literally. None-the-less, while I was putting together this program, it felt like I was doing something very wrong.

package main

import (
	"fmt"
	"math/rand"
	"time"
)

func main() {
	inout := make(chan int, 2)
	divert := make(chan int)
	max := 1000

	fluxcapacitor(inout, divert, max)

	inout <- 1

	for x := range divert {
		fmt.Println("Final Counter:", x)
	}
}

func fluxcapacitor(inout chan int, divert chan int, max int) {
	go incrementor("1", inout, divert, max)
	go incrementor("2", inout, divert, max)
}

func incrementor(s string, inout chan int, divert chan int, max int) {
	for i := range inout {
		if i == max {
			divert <- i
			close(inout)
			close(divert)
		} else {
			inout <- i + 1
			fmt.Printf("Process: %s printing: %d\n", s, i)
		}
		time.Sleep(time.Duration(rand.Intn(10)) * time.Millisecond)
	}
}

Opinions very welcome, or other ways to do this / improvements. Todd’s solution is here, and is far more sensible, although I like that in my code one go routine can take all the load if the other is busy.

What do you think to be wrong about your code?

Ah yes, perhaps should have explained. The function is taking off the same channel it is pushing onto. It locks if you have only one go routine. With two go routines it doesn’t lock unless one of the go routines gets tied up, which is why I added the buffer of two.

Not sure if that’s ok. Maybe it’s more sensible to have two ‘stages’ between the pipes.

Doesn’t your code just fail with “Sending on a closed channel”? I find it wrong that you close the channel “inout” inside the “range inout”.

Your function signature should be “incrementor(s string, in <-chan int, out chan<- int, max int)”. You can use a sync.WaitGroup to keep track of all the incrementor routines that are running.

Yes, that is probably a bad thing. It’s because the last loop is listening for a signal on the channel ‘divert’, and once it captures that it ends. I did originally write it with supplying an in and out channel, but they are the same in this case, so I dropped that. Yes, also dropped the waitgroup from the original code.

It also needs something pushed onto the loop to allow it to get started, hence the

inout <- 1

I’m still unsure whether feeding a channel back around onto itself is a good idea or even useful.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.