rolling-curl icon indicating copy to clipboard operation
rolling-curl copied to clipboard

Fix bug prunePendingRequestQueue returning empty list

Open sudokien opened this issue 10 years ago • 5 comments

Pull request for issue #13

sudokien avatar Mar 28 '15 07:03 sudokien

I see what I've done wrong here, I did the decrement too late. The idea was that if you put the loop at zero, then it would never end until all the items have been accounted for by the break clause. This would work if the while was prefixed, ala while(--$limit), but that's wrong in most other cases.

The easier fix here would be to add

if ($limit <= 0) {
    $limit = -1;
}

right at the function head.

chuyskywalker avatar Mar 29 '15 02:03 chuyskywalker

Hi chuyskywalker,

Yes you're totally right about that. But in my personal opinion I think setting $limit = -1 then having a while ($limit--) will be a bit hard to understand at first sight, since it's an infinite loop and we control iterations by break. If we have while ($countPending--) like in my PR, whoever read this line will know right away that this while loop will loop for every pending requests that haven't been processed. Just a matter of coding style, your fix definitely works (and is shorter).

Can we have this fix in the next release please? :)

Cheers, Kien

sudokien avatar Mar 29 '15 06:03 sudokien

Hi again,

Is there any chance to push this?

sudokien avatar Apr 07 '15 15:04 sudokien

Hi, do you have any updates on this request? I'm using this on production for a while and haven't seen any problems with it.

nhatthm avatar Aug 11 '15 09:08 nhatthm

Hello @chuyskywalker, do you have update on this PR? :) Thanks

nhatthm avatar Dec 30 '15 09:12 nhatthm