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 async.transform #892

Merged
merged 1 commit into from
Oct 25, 2015
Merged

Add async.transform #892

merged 1 commit into from
Oct 25, 2015

Conversation

megawac
Copy link
Collaborator

@megawac megawac commented Aug 24, 2015

Similar concept to https://lodash.com/docs#transform this is a function with several fundamental differences to async.reduce:

  • parallel
  • dynamically interprets memo (signature arr|obj, arr|obj, fn, fn -> arr|obj) which can be useful for recursive cases
  • provides key to iterator function

@qsona
Copy link

qsona commented Aug 25, 2015

Hi, I use lodash.transform very often and think it is also useful on async. neo-async already has the same method, but, a worring factor is that the order of the arguments is not same as that, which places the memo arg at the last. lodash.transform also places it at last so I think it should maintain same order as lodash.transform. How do you think of it?

@megawac
Copy link
Collaborator Author

megawac commented Aug 25, 2015

I'm not sure, I was attempting to be consistent with the reduce argument
order

On Mon, Aug 24, 2015 at 10:00 PM, qsona notifications@github.com wrote:

Hi, I use lodash.transform very often and think it is also useful on async.
neo-async already has the same method, but, a worring factor is that the
order of the arguments is not same as that, which places the memo arg at
the last. lodash.transform also places it at last so I think it should
maintain same order as lodash.transform. How do you think of it?


Reply to this email directly or view it on GitHub
#892 (comment).

@qsona
Copy link

qsona commented Aug 25, 2015

I see. I didn't know that the arguments order of async.reduce is different from that of Array.prototype.reduce.

@aearly
Copy link
Collaborator

aearly commented Aug 25, 2015

👍 once we figure out what the arg order should be. Also seems related to #873

@megawac
Copy link
Collaborator Author

megawac commented Aug 25, 2015

yeah, both array and object maps can be implemented very easily through
transform

On Mon, Aug 24, 2015 at 11:32 PM, Alexander Early notifications@github.com
wrote:

[image: 👍] once we figure out what the arg order should be. Also seems
related to #873 #873


Reply to this email directly or view it on GitHub
#892 (comment).

@megawac
Copy link
Collaborator Author

megawac commented Sep 25, 2015

/cc @suguru03

I think consistency with reduce argument orders is important but I can be swayed on this note

@qsona
Copy link

qsona commented Sep 28, 2015

First of all, I totally agree that the arguments order of async.transform should be same as that of async.reduce.

Next, I think that the arguments order of reduce might be not proper. As that of Array.prototype.reduce is iterator, initialValue, async should follow the JavaScript API rules, so collection, iterator, initialValue, callback is the best order, isn't it?

This order is different from neo-async#transform too.
And of course, the major version up is required if we want to change async.reduce API. If it is not possible, I think it should follow the current order of async.reduce.

@suguru03
Copy link
Contributor

Almost all functions of lodash take optional arguments after required arguments, so I have developed neo-async.transform which take an accumulator of optional argument to last.
Because initial value of async.reduce is optional, I want to place accumulator as the last and think that collection, iterator, callback, [accumulator] order is desirable.
Even if they takes anyway, I'm going to fix to take same argument order with your choice.

@aearly
Copy link
Collaborator

aearly commented Sep 29, 2015

I'd prefer to have it async.reduce(arr, iterator, [accumulator,] callback). Following the node convention of having the callback as the last arg is more important that having optional args last. We can define async.transform with that signature, and then perhaps change async.reduce in a major release?

@qsona
Copy link

qsona commented Sep 29, 2015

+1 😉

@megawac
Copy link
Collaborator Author

megawac commented Sep 29, 2015

Yeah, lets get 1.4.2 shipped soon and put this in v2 with the reduce change

On Mon, Sep 28, 2015 at 8:55 PM, qsona notifications@github.com wrote:

+1 [image: 😉]


Reply to this email directly or view it on GitHub
#892 (comment).

aearly added a commit that referenced this pull request Oct 25, 2015
@aearly aearly merged commit ea357ef into master Oct 25, 2015
@aearly
Copy link
Collaborator

aearly commented Oct 25, 2015

We'll fix the arg ordering later.

@megawac megawac deleted the transform branch April 12, 2016 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants