Count nested operations in StatusCounts

[groovemonkey]

This change adds op.WalkStatuses(), which recursively walks a tree of result ops to get statuses.

[nwchandler]

I'm unclear on why this map would be initialized with a size of 0. 🤔 Would it be better to omit the size so that it doesn't immediately need resized?

[groovemonkey]

Totally right; thanks for catching that!

[groovemonkey]

I changed this to initialize to len(ops), which is the minimum length this will have (if there are no nested ops).

[nwchandler]

We're mutating statuses here, and then returning it at the end. Perhaps we could avoid the mutation, though. What if WalkStatuses took only a results map, and it had an inner function defined, which would take a status map. We could recurse on the inner function and then return what it built up at the end of WalkStatuses. This would, I think, both make the calling of the WalkStatuses easier (no need to create an accumulator) and also avoid accidental mutation.

[nwchandler]

Something like this, perhaps:

func WalkStatuses(results map[string]any) map[Status]int {
	statuses := make(map[Status]int)

	var walkStatuses func(map[Status]int, map[string]any)
	walkStatuses = func(accumulator map[Status]int, results map[string]any) {
		for _, res := range results {
			switch res := res.(type) {

			case map[string]any:
				walkStatuses(statuses, res)

			case Op:
				// Increment and recur
				statuses[res.Status]++
				walkStatuses(statuses, res.Result)

			// End of branch reached
			default:
				continue
			}
		}
	}
	walkStatuses(statuses, results)
	return statuses
}

[nwchandler]

walkStatuses could also just be an unexported function as well - the main idea is, can we avoid mutation in the public API?

[mkcp]

Not certain it's the ideal implementation, but it does let us pass in partially constructed accumulators for testing or for more elaborate concurrent reducers. You could reasonably do the same thing without passing in an accumulator though, treating it as the steps: split/partition results -> map over result subset and return map[Status]int -> merge all map[Status]int. Also, the accumulator is passed by value so we don't have to worry about changes from outside goroutines.

[mkcp]

I was mistaken! maps are like channels not slices and get passed by reference even without an explicit pointer!

[groovemonkey]

Thanks -- used @nwchandler 's inner-closure version of this function. Let me know if that's good to go!

[nwchandler]

Looks good to me!