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

Branches Does not preserve order #215

Open
Millsky opened this issue May 9, 2019 · 2 comments
Open

Branches Does not preserve order #215

Millsky opened this issue May 9, 2019 · 2 comments

Comments

@Millsky
Copy link

Millsky commented May 9, 2019

First off, I would like to say great work on this library, the documentation is fantastic and it makes dealing with complex objects easy.

My issue resolves around the order in which the branches are visited.
for example:

L.branches(1,2,3) produces an object with the following keys. [1,2,3].
L.branches(3,2,1) produces an object with the following keys. [1,2,3]

I would expect L.branches(3,2,1) to visit the keys in the order in which they were provided as arguments, as opposed to the object key order.

I would propose using a Map to preserve the order of the arguments.

@polytypic
Copy link
Member

Thanks! Just to recap, the current documentation says that

L.branches(p1, ..., pN) is equivalent to L.branch({[p1]: [], ..., [pN]: []})

which means that the keys are examined in object key order. The proposal is to change the order to match the order of the arguments instead.

🤔

Yeah, I think that change makes sense. It does likely require a bit more code to build the map and it is technically a breaking change, but unlikely to affect most users.

Did you run into a problem due to this?

I could imagine e.g. one using L.forEach with L.branches and being surprised due to the traversal order.

@Millsky
Copy link
Author

Millsky commented May 9, 2019

@polytypic - I was using branches along with a disperse, I had a collection of ids to update.

L.branches(...formIds) the formIds were strings of numbers for example: ['1256', '9807', '123'].

Then I would use L.disperse(myBranchesTraversal, [1,2,3], {}); I guess this is another instance of JS behavior being a bit unexpected. As, when the string keys are put into an object they are sorted and then traversed in that order.

Another option may be to create another orderedBranches that maintains the argument or array order as opposed to the object key order, that way it wouldn't break any users that are relying on the object key order.

Thanks for the quick response by the way!

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

No branches or pull requests

2 participants