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

CRM_Queue_Queue - Add 'getStatistic($name)'. Deprecate 'numberOfItems()' #24452

Merged
merged 4 commits into from
Sep 4, 2022

Conversation

totten
Copy link
Member

@totten totten commented Sep 3, 2022

Overview

(This is an off-shoot from #24408.)

Should numberOfItems() tell you the total number of items -- or the number of ready items? Trick question. They can tell you different things:

  • When implementing a task-worker (which loops through available tasks, and stops when there's nothing to do), you'd use ready items.
  • When implementing existsQueue() (which tells if you the queue has anything at all), you'd use total items.

This trick-question is why different implementations have computed numberOfItems() in different ways.

This patch resolves the trick-question by deprecating numberOfItems() and replacing it with getStatistic($name).

Before

  • numberOfItems() is ambiguous

After

  • numberOfItems() is ambiguous
  • getStatistics($name) replaces it

eileenmcnaughton and others added 4 commits September 2, 2022 17:16
Should `numberOfItems()` tell you the _total_ number of items -- or the
number of _ready_ items?  Trick question.  They can tell you different
things:

* When implementing a task-worker (which should start/stop based
  on available tasks), you'd use _ready_ items.
* When implementing `existsQueue()` (which tells if you the queue
  has anything at all), you'd use _total_ items.

This trick-question is why different implementations have computed
`numberOfItems()` in different ways.

This patch resolves the trick-question by deprecating `numberOfItems()`
and replacing it with `getStatistic($name)`.
@civibot
Copy link

civibot bot commented Sep 3, 2022

(Standard links)

@civibot civibot bot added the master label Sep 3, 2022
public function numberOfItems() {
return count($this->items);
public function getStatistic(string $name) {
$ready = function(): int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm - this inline function threw me at first - would be more readable as a separate function but OK - it's a short enough function it doesn't matter

Copy link
Member Author

Choose a reason for hiding this comment

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

PHP visibility levels:

  • public
  • protected
  • private
  • so_private_you_can_t_even_see_it

Copy link
Contributor

Choose a reason for hiding this comment

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

lol - yes it is VERY private

@eileenmcnaughton eileenmcnaughton merged commit 180ae18 into civicrm:master Sep 4, 2022
@totten totten deleted the master-queue-stats branch September 4, 2022 22:59
@totten totten added the has-test label Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants