Why are people malcontent about this?

[nematode (Antverpen)]

Hello, I have started learning golang recently. Doing some puzzles I come up with this code:

package main

import (
	"fmt"
)


func main() {
	
 	s := []int{1,2,3,4,5}
	
	count := len(s) 
	finishSignal := make (chan bool, count)
	
	for i := range s {
		t := i
		go func () {
			s[t] += 1
			finishSignal <- true
		} ()
	}
	
	for _ = range s {
		<- finishSignal
	}
	fmt.Print(s)
}

This example code uses a scatter-gather pattern I assumed is quite common in go. You split the work, you make workers, and await results. To my surprise, many people have said that this thing is horrible and that I should use predefined utilities from std lib, WaitGroup to be precise.

My question is ‘what’s wrong with this code?’ Is it actually bad?

[CurtGreen (Curtis Allyn Green)]

I’ll tell you why I think it is not good for the behavior you want. Using a channel in this way isn’t clear what you are doing until you really think about it, but a WaitGroup is clear in it’s usage and makes reading the code easier.
https://gobyexample.com/waitgroups

[Massimo.Costa]

I’m very beginner in Go but from my understanding channels “can” be used as synchronization primitives but their primary intent is to share information between goroutines.

WaitGroup is deigned instead to wait until a set of goroutines finishes.
I’d use channels only if the goroutines returns some results

[Dean_Davidson]

If there’s an idiomatic way of doing things, it will reduce the overhead of developers who have to read your code in the future if you use it. In your example, I had to ask myself “OK what is the intent here?”. If you had used sync.WaitGroup, I would have been able to easier understand your intent.

Let’s say you are code reviewing my check-in and it looks like this:

package main

import (
	"fmt"
)

// A contrived example...
func main() {
	for i := 1; i <= 10; i++ {
		DoIf(i%2 == 0, func() {
			fmt.Println(i, "is even")
		})
	}
}

// DoIf runs f if value is true.
func DoIf(value bool, f func()) {
	if value {
		f()
	}
}

You have to do a little more parsing to understand what DoIf is doing. It’s also more verbose (as is your example code). If I refactored to the following, it would be easier to understand, right?

if i%2 == 0 {
	fmt.Println(i, "is even")
}

Idiomatic code is always going to be better for other maintainers. sync.WaitGroup is the idiomatic to wait for jobs to finish and thus you should use it. At least that’s my $.02.