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

add min and max macros #373

Open
jlami opened this issue Aug 4, 2017 · 7 comments
Open

add min and max macros #373

jlami opened this issue Aug 4, 2017 · 7 comments

Comments

@jlami
Copy link

jlami commented Aug 4, 2017

You do have Math.min/max, but those only work on non-array arguments. Ember.computed.min/max work on arrays. You do have a reduce, but I don't want to use that each time I want a min or max

I don't know if these should become array.min and array.max?

@kellyselden
Copy link
Owner

I like the idea of array.min and array.max.

@jlami
Copy link
Author

jlami commented Aug 4, 2017

Feels a bit weird though to have two versions of min and max. I don't know if the new versions could also be made to work like sum? That looks to be able to accept keys that resolve to single values as well as arrays. But the code would have to be adopted to pass in new initial values for the reduceKeys helper I think?
A faster way would probably be to use array.reduce as a helper. But that would lead to the user having to know which min/max version to use in which case. I could probably whip this up if you need any help. The reduceKeys one might be a bit much for me, just started looking at your internal code...

@kellyselden
Copy link
Owner

Another option could be a rest operator macro:

value: math.min(array.rest('myArray')) // Math.min(...myArray)

@jlami
Copy link
Author

jlami commented Aug 4, 2017

Would that really work? As I see it the math.min is just a pass through to Math.min. What would the array.rest do to make this work on the input side? The (curried)computed that math.min uses is still just giving the parameters the normal way right?
For normal functions the spread works because it indicates that the function has to be called differently. I don't see how this would work in this case.

@kellyselden
Copy link
Owner

It wouldn't work without a lot of changes. I was just throwing it out there.

@jlami
Copy link
Author

jlami commented Aug 4, 2017

Ah, ok. I see. I'll have to think about it. Could make it more uniform. But I don't know if there are really more use cases for the array.rest.

@jlami
Copy link
Author

jlami commented Aug 4, 2017

Maybe an apply(func, arrayKeyOrComputed) could be a simpler alternative? The spread is just that. The normal computed could be seen as the func.call counterpart I think? So it might also be called computedApply maybe?

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

No branches or pull requests

2 participants