BlakeGuilloud / ganon

A javascript library
MIT License
92 stars 196 forks source link

Implement distanceN. #785

Closed crhallberg closed 5 years ago

crhallberg commented 5 years ago

Closes #784

crhallberg commented 5 years ago

I haven't added a new skeleton method, but I do think that mapArray isn't working the way most developers would expect a map function to work. You can see this in the test where the callback is iterating through an entire array, not just an element: https://github.com/BlakeGuilloud/ganon/blob/master/test/mapArray.test.js#L6

mtso commented 5 years ago

@crhallberg it would be helpful to show how you think it should work by updating the test to fail the existing implementation.

Just to clarify, though: are you expecting mapArray to work like Array.map?

crhallberg commented 5 years ago

That is correct. Right now mapArray is passing the whole array to the callback, which is not what I expected when I was looking to use map for distanceN.

crhallberg commented 5 years ago

mapObject is iterating over the items: https://github.com/BlakeGuilloud/ganon/blob/master/lib/mapCollection.js#L23

I think it may be a case of the implementer misinterpreting the doc comment at the top of the file: "Implement your own map function, obviously without using JavaScript's map function"

crhallberg commented 5 years ago

I mean... I'm probably taking this too seriously...

ktilcu commented 5 years ago

@crhallberg thanks for pointing that out! i made a PR to fix it.

crhallberg commented 5 years ago

Added a skeleton to pass this on. I think we're ready to go!

ktilcu commented 5 years ago

Looks goo to me! Please add an issue for your new skeleton. Happy Hacktobering!