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

make PriorityQueue array protected rather than private #140

Open
trajano opened this issue Apr 11, 2021 · 5 comments
Open

make PriorityQueue array protected rather than private #140

trajano opened this issue Apr 11, 2021 · 5 comments

Comments

@trajano
Copy link

trajano commented Apr 11, 2021

This will allow us to extend the existing priority queue to make methods that change to the underlying structure for example "remove" to remove entries from the promise queue for scenarios like an image component getting unmounted so there's no point in keeping the promise of downloading the image.

Right now I am copying the class and adding the new method

  removeWithPredicate(predicate: (entry: PriorityQueueOptions & { run: RunFunction }) => boolean): void {
    let i, j;

    for (i = 0, j = 0; i < this._queue.length; ++i) {
      if (predicate(this._queue[i])) {
        this._queue[j] = this._queue[i];
        ++j;
      }
    }

    while (j < this._queue.length) {
      this._queue.pop();
    }
  }
  

based off https://stackoverflow.com/a/54270177/242042

@sindresorhus
Copy link
Owner

That's very much intentional. Allowing subclassing makes pretty much any internal behavior public. It's not something I would want to commit to. However, I'm happy to see a "remove" like method added.

@trajano
Copy link
Author

trajano commented Apr 11, 2021

It would be easier than to get people to fork it off. But I've gotten most of it done now. It's just a matter of figuring out how to call the remove method I have written above. Without doing a full copy of the class. The thing that is confusing me is how to convert the item I want to remove to something that would be a RunFunction and also Task does not seem to be exported.

Looking through the code it's not feasible to use the run to check whether the item has to be removed from the queue as run is built as an anonymous function and we can't easily map that to the original function. So I modified the code I had above to take in a predicate to evaluate whether to delete the record

What I think it needs as well is to add the fn variable into the PriorityQueueOptions object. That way the predicate can check whether the entry should be removed. I noticed I used run directly so I am changing that to match the queue data

Going to continue the remove discussion on #76

@Richienb
Copy link
Collaborator

Maybe we could imitate the prototype functions of Set within the class. Would be breaking through.

@Richienb
Copy link
Collaborator

@sindresorhus What do you think? This would allow the queue to support features like #76.

@sindresorhus
Copy link
Owner

Maybe we could imitate the prototype functions of Set within the class. Would be breaking through.

Not sure I see the benefit of that?

I think just adding a .remove() method to solve this well-defined problem is better.

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

3 participants