Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

These aggregation calls would make a good use case for map/reduce.. map/collect. #8

Open
shinjonathan opened this issue Nov 3, 2020 · 1 comment

Comments

@shinjonathan
Copy link
Contributor

for(let contribution of contributions) {

@KianBadie
Copy link
Contributor

Thank you for the recommendation Jonathan 👍 Sorry for getting to this so late, I started to give these a deeper look today. I thought they were going to be straightforward and that I would knock them out today, but I had a question about implementing map/reduce.

I had trouble understanding how map/reduce would be implemented in the aggregation functions. I'll frame the question around the aggregateContributors function, as it is a little bit simpler than the aggregateContributions function. If map was called on every contributor, the only way I can see reduce being incorporated into that is by calling reduce on the current contributor in order to loop through the contributor array and aggregate the contributions when the contributor name matches the current contributor. But this seems like a "nested for loop" in a way, where reduce is being called for every iteration of map. Unless you could remove elements during the process, but that does not seem right either. Is there something I am missing in understanding this?

I started to see how reduce could be used without map. With the accumulator for reduce being the contributorDictionary object itself. That solution did not seem to reduce any code though. Even though it didn't reduce code, do you find that it has semantic value? As in it is clearer to a reader of the code what is happening? If so, I would find that a compelling reason to add that to these functions. But if not, I would prefer to keep it as is for the sake of convenience.

Lastly, I was not sure what "map/collect" meant. Was "collect" meant to refer to something else?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

2 participants