Correct way of merging channels

Hi, everybody I have a problem with merging channels, I am trying to solve a multithreading task, but testing system always gives Wrong Answer on any of my attempts. So I have code which should read values from both channels and put f(a) + f(b) to the output, n times. My basic idea presented below. It seems to work for my tests but can’t pass even the first in test system. May someone please point out mistake in my logic or give any ideas on possible solutions. Thanks!

func merge(f func(int) int, ch1 <-chan int, ch2 <-chan int, out chan int, n int)  {
	go func() {
		defer close(out)
		for i := 0; i < n; i++ {
			select {
			case v := <-ch1:
				out <- (f(v) + f(<-ch2))
			case v := <-ch2:
				out <- (f(v) + f(<-ch1))
			}
		}
	}()
}

Is it guaranteed that both channels will always have the same number of items put in? Why not just read them in sequence?

a :=<- ch1
b :=<- ch2
out <- f(a) + f(b)

I’ ve tried this but it didn’t work:( The task description only constraint is that merge function should be non-blocking, it doesn’t mention if the channels would be the same size or not. So I am struggling with it 2 days, trying to realise which corner-cases could break my code

I don’t know the full description, though if channels don’t have at least both n items, your merge can’t work. Also, what exactly is meant by “non blocking”? Unless you use a select with a default branch, reading a channel always blocks.

I’ve tried to add an empty default to my code, but unsuccessfully
Full Description:

You need to write function func merge(f func(int) int, ch1 <-chan int, ch2 <- chan int, out chan<- int, n int) in package main.

Description:

n times make the following

* Read one value from each channel in1 and in2, we will call it x1 and x2.
* Calculate f(x1) + f(x2)
* Write the result to out

Function merge must be non blocking , I.e. immediately returning the control

Function f may work for a long time or make some calculations

The bullet points describe exactly what I did in my code.

Though the hint that f might be long running, makes me assume that they also want you to do those in a go routine to avoid timeouts in the producers.

Also, I’d not close a channel that I do not “own”…

Everything I am trying I do in goroutine so I assumed that snippet you suggested wrapped in goroutine:) Yeah, I’ve tried not to close the out channel.I’ ve also tried to check whether any of the channels finished and if yes to stop cycle. But it doesn’t work anyway that’s why I am in desperation trying to fix it. I don’t really see mistake in my solution, moreover I am
almost convinced myself that probably there is a mistake in testing system…

When I was at university and we had doubts about functionality of tests or questions about the exercise, we could always go to the teachers and ask.

If you were able to convince them you put enough effort into understanding it yourself they looked at the code with you and usually helped you to find the problem. I hope your teachers are the same kind of people.

@Carramba I agree with NobbZ comment that checking with the teacher is probably a good idea here. I created an implementation that I think meets the description’s requirements, but I wouldn’t use this code, personally: https://play.golang.org/p/dMETV35QtKU

NobbZ also asked if it’s guaranteed if the channels have the same number of elements- the implementation I made will not work correctly if:

  • One or both of the input channels produce fewer than n values passed to merge. The way I wrote it, you’ll get a panic. If you remove the ok value from the channel receive, it’ll block forever.
  • One or both the input channels produce more than n values. the goroutines and channels will never be garbage collected.
  • You can’t range over the output channel unless you close it in merge (or perhaps somewhere else) but that can get complicated if you need to transfer ownership of the output channel. NobbZ mentioned already that it’s probably a code smell to close a channel your function is given.

Because of the “n times make the following” requirement, you have to track that n value all the way from the producers, the merge, and the consumer(s). If you remove that requirement, you can change the implementation to not hang/panic if the values don’t all match up.

Hi, thank you for helping. You gave me idea to wrap writing to output channel into goroutine, so I will try to play around with this and if it won’t help, I will for sure contact my teacher and get advice.) Thank you both for help!

Do yourself a favor. Get a copy of “Concurrency in Go: Tools and Techniques for Developers”, by Katherine Cox-Buday. Read it, and do the exercises. I have done that, and it really opened my eyes about how to do concurrency in Go.