Conversation
raklaptudirm
left a comment
There was a problem hiding this comment.
Please explain how your implementation is better. I for one, think the previous one was the better one.
|
If the goal is learning and understanding what the search for the minimum is - the previous implementation is better (although, as it seems to me, everyone already understands what the search for the minimum means) If we need a more 'production-ready function' that is able to work with different structures (iterables for example) and with different ways to search for the minimum itself, then mine is better. Finding the minimum is not always looking for the smallest number |
|
This repo is not for production ready code, but for self explanatory code understandable for beginners of the programming language. Throwing iterators at them is not a good idea imo. |
|
I like this implementation. Maybe as an alternative test('given array of objects then min accessor is found', () => {
const array = [
{ name: 'Item #1', price: 1.0 },
{ name: 'Item #2', price: 0.0 },
{ name: 'Item #3', price: -1.0 }
]
expect(findMin(array, _x => _x.price)).toBe(-1)
expect(findMin(array, _x => _x.name)).toBe('Item #1')
})The tests pass $ npx jest --no-cache Maths/test/FindMin.test.js
PASS Maths/test/FindMin.test.js
findMin
✓ given empty array then min is undefined (2 ms)
✓ given single value array then min is found (1 ms)
✓ given array then min is found
✓ given empty generator then min is undefined
✓ given generator then min is found
✓ given string generator then min string length is found
✓ given array of objects then min accessor is found (1 ms)
Test Suites: 1 passed, 1 total
Tests: 7 passed, 7 total
Snapshots: 0 total
Time: 1.147 s
Ran all test suites matching /Maths\/test\/FindMin.test.js/i.Edit: Expanded the test a bit. |
I agree. Iterators and generators are really difficult topics to understand and even more to explain. I think it is far more easier and useful to explain a simple for-loop (as seen in the previous implementation) than explaning iterators. So in my opinion and for the target audience of this repo, I think the previous implementation is better. |
|
@trasherdk I completely agree on the fact that this implementation is faster and more useful than the current one, but thats not my point. |
|
@raklaptudirm I get your point. Don't complicate stuff 😃 I just liked the implementation, and the use case I demonstrated. |
|
We could add this as a separate implementation though. |
|
That would work too 😃 What do you say @RuSaG0 ? |
|
@trasherdk |
|
@RuSaG0 Make the changes and I will review them. |
improve find Min method
if no params to calc min => it's not error, it's undefined/null
work with generators, custom selector fn