Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

Add priority parameter #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Pierozi
Copy link
Member

@Pierozi Pierozi commented Jul 25, 2016

Hello,

I wrote this PR first before any issue because i had to write this patch for move forward on customer project.
I don't know if the state of art is good enough, but seems good to expose as a PR.

My case : I use Hoa\Exception and the power of event for handle correctly the translation of exception into a dedicated Library. But then i use this library into an App, and this app need be able to plug his own callable before library for catch specific exception and throw another dedicated to the library.

My solution was to add an integer argument to attach method for define priority level, higher is most important and called first.

https://continuousphp.com/git-hub/Pierozi/Event/build/6019f777-ed30-4923-882e-42a213021ff0/output

$a = $this->_priorities[$a];
$b = $this->_priorities[$b];

/*if (70000 <= PHP_VERSION_ID) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove this comment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, first try to make compatibility with spaceship operator but it's parser issue.

@Pierozi
Copy link
Member Author

Pierozi commented Jul 29, 2016

@Hywan After a bunch of test the big deal with Spl*is they are all pure queue implementation, that mean the first foreach meet will dequeue all the values. and the second issue was it's bit complicated to mix priority with xcallable hash.

Otherwise we can still create our own class with ArrayAccess and Iterator pattern for have consistent solution. This should be part of Hoa\Consistency\XcallableHeap because it's Xcallable related things.

@Hywan
Copy link
Member

Hywan commented Aug 4, 2016

@Pierozi The best solution would be to create the Hoa\Heap library. Would you like to go with this solution 😋?

@Pierozi
Copy link
Member Author

Pierozi commented Aug 4, 2016

That could be good, but that must be a super Heap, we need to be able to specify a key and priority for each item, and have parameter for enable or not dequeue when iterate.

What thinks the others @hoaproject/hoackers ?

@osaris
Copy link
Member

osaris commented Aug 8, 2016

From a pure architectural point of view, I think that creating a Hoa\Heap library is a good idea and that could include what @Pierozi need for this use case (priority etc) ?

@Pierozi
Copy link
Member Author

Pierozi commented Aug 12, 2016

@osaris Yes :) I'm ready for write Hoa\Heap if it's good.
@Hywan do you lead to create new repository ?

@Hywan
Copy link
Member

Hywan commented Aug 15, 2016

@Pierozi Create your own library, and we will fork it after.

@Pierozi
Copy link
Member Author

Pierozi commented Aug 19, 2016

FYI, development in progress in repo https://github.com/Pierozi/HoaHeap

@Hywan
Copy link
Member

Hywan commented Aug 22, 2016

Nice :-). I don't know how we can do the tracking of the progression. How would you be comfortable?

@Pierozi
Copy link
Member Author

Pierozi commented Aug 22, 2016

I guess the best should create empty repo and then I can do a PR, because fork my repo is not good idea repo should belong to Hoa. Or we can just define an issue on my repo until community agree with library.

@Hywan
Copy link
Member

Hywan commented Aug 23, 2016

@Pierozi I think the second solution is the best for now.

@Pierozi
Copy link
Member Author

Pierozi commented Aug 28, 2016

The code of library are finished with the support of Min and Max, as common Heap does, and the performance / quality satisfies me. I need write api explication and demo in Readme.

There is the issue Pierozi/HoaHeap#6

@hoaproject/hoackers reviews always welcome.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants