Coherent way of maintaining the index value

adrian-wozniak commented 3 years ago

This PR fixes the way of checking if index property is defined and the way how index is calculated when registering the node.

Description

Currently index property existence was simple verified be checking if such property is "truthy" or "falsy". For number it's wrong approach because '0' is "falsy", but on the other hand is a valid (defined) number.

It is also not possible to insert child at a given position. If given index value is already occupied by other child, than both children shares the same index value, which results in a bit weird behavior while searching for next or prev node. In such case the moment of registering takes precedence. To deal with that issue, the new property isCoherentIndex is introduced. A node that has such property defined, keeps indices of its children coherent and compact. The child is inserted at a given index position and all other children indices that are greater or equal to index of just registering child are shifted up by one. If index of the registering child is greater than current size of children list, than child is appended and its index is set accordingly as last.

Motivation and Context

Issue #84, #88

How Has This Been Tested?

Fix was tested by unit tests and manually in app using LRUD library where such issue was found.

Screenshots (if appropriate):

No screenshots.

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[x] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[x] My change requires a change to the documentation.
[x] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

adrian-wozniak commented 3 years ago

@jordanholt Could I ask you to take a look on that?

jordanholt commented 3 years ago

Given it some thought and we think that coherent indexes should be the default behaviour, could you rework the PR to remove the isIndexCoherent flag and we can release this as part of a major version.

adrian-wozniak commented 3 years ago

@jordanholt That's even better! Thank you!

bbc / lrud