pradosoft / prado

Prado - Component Framework for PHP
Other
187 stars 70 forks source link

#908 Adding items with [] adds by new index in TMap, TPriorityMap preserves numeric keys #909

Closed belisoful closed 1 year ago

belisoful commented 1 year ago

With unit Tests update.

belisoful commented 1 year ago

This is insufficient. More unit tests need to be written.

belisoful commented 1 year ago

I've been researching this issue.

With this new merge-issue, TMap will work properly with $map[] = $item; adding new items at the proper incrementing index. It will also preserve out of order indices and take the max numeric +1 for the next index when some numbers are skipped (eg $map[999] = $itemX), and then a $map[] = $item2 happens (eg it will place in the next location 1000). Updated unit tests. This is simple and works.

TPriorityList has no issues with adding at the null key (at the default priority, in priority order). But needs to be cleaned up. the append value (internally) was false and should be null. Subclasses are updated, likewise. Updated unit tests. using a merge to combine the items in each priority does properly reorder and flatten the list items in the various priority to an array [0..n] in priority and list order.

Currently, TPriorityMap is re-indexing numeric and compressing them all into 0 .. n on array_merge. All numeric information is lost, indexing numerics in a TPriorityMap is entirely wonky. This is not what it should be doing with numeric keys.

If TPriorityMap adds an internal index counter for max numerics + 1, uses array_replace rather than array_merge and new items without key are given the internal counter as the key, it can work.

It should be noted that indexing in a TPriorityMap is not the same as indexing in a TPriorityList. TPriorityList will have its items in order of priority, in a list. An item can change index (key) if a higher priority item is inserted before the index being "watched".

Contrary, the TPriorityMap could have key "0" be priority 100, key "1" be priority -3, key "2" be priority 10. There is no "reindexing" of map items for priority (like TPriorityList), that was an error. The index/key will stay the same regardless of any inserts at any priority.

I used the code from TPriorityList to make TPriorityMap and didn't change the array_merge to array_replace to compensate.

belisoful commented 1 year ago

Ready for review and merge.

This fixes the numeric keys in TPriorityMap from being thrashed like a list. The code from TPriorityList was copied for TPriorityMap (long ago) and array_merge never got changed to array_replace... it works for non-numerics.

belisoful commented 1 year ago

I have one more unit test to include.

belisoful commented 1 year ago

OK. This has everything.

Ready for review and merge.