Add CbpStrategy accepting OBP params #517
Conversation
ecoologic
force-pushed
the
RED-1984-order
branch
from
9cbb72b
to
0ad78a2
Compare
ecoologic
changed the title
| $params['sort_order'] | ||
| ); | ||
| return $params; | ||
| } |
There was a problem hiding this comment.
so now the CbpStrategy knows a lot about OBP. I thought of extracting it, and I did,
but it leads to a lot of indirection. In the end I prefer to accept some duplication and imperfect encapsulation in favour of simpler code.
| @@ -0,0 +1,67 @@ | |||
| # Upgrade guide | |||
There was a problem hiding this comment.
There's a little duplication in the info below, but I think it's worth giving a TLDR summary.
| foreach ($ticketsIterator as $ticket) { | ||
| process($ticket); // Your implementation | ||
| foreach ($iterator as $ticket) { | ||
| echo($ticket->id . " "); |
There was a problem hiding this comment.
I now prefer code that is fully testable, without having to define process
ecoologic
force-pushed
the
RED-1984-order
branch
2 times, most recently
from
22ee2fa
to
3976fad
Compare
ecoologic
force-pushed
the
RED-1984-order
branch
from
3976fad
to
01a87d6
Compare
UPGRADE_GUIDE.md
Outdated
|
|
||
| **OBP (Offset Based Pagination)** is quite inefficient, and increasingly so the higher the page you fetch. Switching to **CBP (Cursor Based Pagination)** will improve your application performance. OBP will eventually be subject to limits. | ||
|
|
||
| When in OBP you request page 100, the DB will need to index all 100 pages of records before it can load the rows for the page you requested. |
There was a problem hiding this comment.
Should we rephrase this? Maybe something like:
When using OBP, if you request page 100, the DB will need to index all records that proceed it before it can load the rows for the page you requested.
UPGRADE_GUIDE.md
Outdated
| } | ||
| ``` | ||
|
|
||
| If this is your situation, **you need to change sorting order** to a supported one. |
There was a problem hiding this comment.
Possibly change this to:
If this is your situation, you will need to change the sorting order to a supported one.
UPGRADE_GUIDE.md
Outdated
|
|
||
| ## The new iterator | ||
|
|
||
| Your best solution to implement CBP is to use the newly provided iterator on your resources, for example: |
There was a problem hiding this comment.
Nitpickiest nitpick. I
The most elegant way to implement CBP....
if you use the iterator with a CBP resource, you might not need to update the sort and pagination parameters.
This starts the upgrade guide, but please don't consider it complete or final. More work will come in that area.
I suggest reviewers to use Split view.