Skip to content

Change channel priority sort#37

Merged
ajacksified merged 1 commit intoajacksified:masterfrom
tbusser:feature/channel-priority
Mar 22, 2018
Merged

Change channel priority sort#37
ajacksified merged 1 commit intoajacksified:masterfrom
tbusser:feature/channel-priority

Conversation

@tbusser
Copy link
Contributor

@tbusser tbusser commented Sep 3, 2014

This fixes issue #15 where a subscriber with an initial priority of 9
winds up being called after a subscriber with priority 1 which was later
added.

Instead of manually splicing the subscriber into the array of
subscribers the addSubscriber and setPriority and changing the value
of the priority property, subscribers are now sorted using the sort
function of the array with a custom compare function.

This compare function will sort the subscribers based on their priority
property on the options object in descending order. The custom sort
method doesn't not manipulate the value of the priorityproperty giving a
much more predictable result.

The unit tests for the priorities have been update to better test if the
sort works as expected. An additional unit test specifically for
issue #15 has been added.

This fixes issue ajacksified#15 where a subscriber with an initial priority of 9
winds up being called after a subscriber with priority 1 which was later
added.

Instead of manually splicing the subscriber into the array of
subscribers the `addSubscriber` and `setPriority` and changing the value
of the priority property, subscribers are now sorted using the `sort`
function of the array with a custom compare function.

This compare function will sort the subscribers based on their `priority`
property on the `options` object in descending order. The custom sort
method doesn't not manipulate the value of the priorityproperty giving a
much more predictable result.

The unit tests for the priorities have been update to better test if the
sort works as expected. An additional unit test specifically for
issue ajacksified#15 has been added.
@ajacksified ajacksified merged commit d27b5bd into ajacksified:master Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants