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

Want a way to iterate through a tree from right to left #107

Closed
xinxinw1 opened this issue Nov 27, 2020 · 3 comments
Closed

Want a way to iterate through a tree from right to left #107

xinxinw1 opened this issue Nov 27, 2020 · 3 comments
Labels
api feature This issue is about adding methods to the API

Comments

@xinxinw1
Copy link

Hi @aureooms, I've been using your Finger tree javascript implementation in an online calculator that I'm building and so far it has been really good and easy to use. However today I found that I needed to iterate through the items of a tree from right to left and realized this was gonna be really difficult to do efficiently without changing your fingertree code.

Would it be possible to add something like an Tree#reverseIter() method or something like it to the tree API that'll efficiently iterate through the tree from right to left like Tree[Symbol.iterator]() does from left to right?

I'm guessing part of the implementation will probably look something like

Deep.prototype.reverseIter = function* () {
	yield* this.left.reverseIter();
	for (const node of reversed(this.middle)) yield* node.reverseIter();
	yield* this.right.reverseIter();
};

but I'm not familiar enough with the code to be sure of the details.

@make-github-pseudonymous-again
Copy link
Member

Hi @xinxinw1! After a quick glance I think that you would need at least a reverseIter method for each possible implementation of a tree: Deep, Single, Empty, and Lazy. Using reversed on this.middle would be quite bad (with an implementation similar to https://github.com/aureooms/js-itertools/blob/main/src/map/reversed.js). The code for Deep should look like

Deep.prototype.reverseIter = function* () {
	yield* reversed(this.right);
	for (const node of this.middle.reverseIter()) yield* reversed(node); 
	yield* reversed(this.left);
};

Note that we iterate first on the right digit then on the left digit.

To achieve the best performance I guess that digits and nodes could also get an implementation of reverseIter. But for a first implementation the only part where it feels necessary is for this.middle since its size can be arbitrary (digits and nodes have size at most 4).

Just a sanity check, following the definition of a tree:

data Tree x = Empty
            | Single x
            | Deep ( Digit x ) ( Tree ( Node x ) ) ( Digit x )

In the sketched implementation of Deep.prototype.reverseIter above:

  • yield* reversed(this.right); yields elements of type x in reverse order.
  • for (const node of this.middle.reverseIter()) yields elements of type Node x in reverse order.
    • yield* reversed(node); yields elements of type x in reverse order.
  • yield* reversed(this.left); yields elements of type x in reverse order.

Before this gets implemented here, you can try to write a function that achieve the same. You need to branch depending on the tree implementation:

const reverseIter = ( tree ) => {
    switch (tree.constructor) {
        case Empty: return reverseIterEmpty();
        case Single: return reverseIterSingle(tree);
        case Deep: return reverseIterDeep(tree);
        case Lazy: return reverseIterLazy(tree);
        default: throw ...;
    }
};

Then implement each case. You could even add digits and nodes to the switch once it works (One, Two, Three, Four, Node2, Node3).

Can you try this?

@make-github-pseudonymous-again make-github-pseudonymous-again added the feature This issue is about adding methods to the API label Jan 8, 2021
@make-github-pseudonymous-again
Copy link
Member

Note that it makes sense to implement this reverse iteration since monoid addition is associative.

@make-github-pseudonymous-again
Copy link
Member

make-github-pseudonymous-again commented Mar 25, 2021

@xinxinw1 You can now use for (const el of tree.reversed()) .... It currently returns an IterableIterator. I may make it return a tree in the future but even then for ... of ... will still work. If you want a tree with the items in reverse you can use from(measure, tree.reversed()).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api feature This issue is about adding methods to the API
Projects
None yet
Development

No branches or pull requests

2 participants