Skip to content

Commit

Permalink
Merge pull request #192 from rimbu-org/fix/sortedset-getatindex
Browse files Browse the repository at this point in the history 
fix(sorted): fix sortedset and sortedmap builders not correctly clearing their source when modified
  • Loading branch information
@vitoke
vitoke authored Dec 9, 2023
2 parents 54520fe + 196cb90 commit 3e19056
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
18 changes: 18 additions & 0 deletions deno_dist/sorted/common/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,8 @@ export abstract class SortedBuilder<E> {
}
}

// check whether child at given `childIndex` has gotten too small
// get from neighbouring children if it is too small so child increases
normalizeChildIncrease(childIndex: number): void {
const child = this.children[childIndex];
if (child.entries.length >= this.context.minEntries) return;
Expand All @@ -869,6 +871,7 @@ export abstract class SortedBuilder<E> {
// cannot shift
if (undefined !== leftChild) {
// join left
leftChild.source = undefined;
const [down] = this.entries.splice(childIndex - 1, 1);
leftChild.entries.push(down);
leftChild.entries = leftChild.entries.concat(child.entries);
Expand All @@ -882,6 +885,8 @@ export abstract class SortedBuilder<E> {
}

// join right
rightChild.source = undefined;
child.source = undefined;
const [down] = this.entries.splice(childIndex, 1);
rightChild.entries.unshift(down);
rightChild.entries = child.entries.concat(rightChild.entries);
Expand All @@ -900,6 +905,8 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length < leftChild.entries.length)
) {
// get from left
child.source = undefined;
leftChild.source = undefined;
child.entries.unshift(this.entries[childIndex - 1]);
child.size++;
this.entries[childIndex - 1] = leftChild.entries.pop()!;
Expand All @@ -916,6 +923,8 @@ export abstract class SortedBuilder<E> {
}

// get from right
child.source = undefined;
rightChild.source = undefined;
child.entries.push(this.entries[childIndex]);
child.size++;
this.entries[childIndex] = rightChild.entries.shift()!;
Expand All @@ -929,8 +938,11 @@ export abstract class SortedBuilder<E> {
}
}

// check whether child at given `childIndex` has gotten too large
// shift to neighbouring children if it is too large so child decreases
normalizeChildDecrease(childIndex: number): void {
const child = this.children[childIndex];

if (child.entries.length <= this.context.maxEntries) return;

const leftChild = this.children[childIndex - 1];
Expand All @@ -943,6 +955,7 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length >= this.context.maxEntries)
) {
// need to split child
child.source = undefined;
const index = (child.entries.length >>> 1) + 1;
const preSize = child.size;
const rightEntries = child.entries.splice(index);
Expand Down Expand Up @@ -980,6 +993,9 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length >= leftChild.entries.length)
) {
// shiftleft
leftChild.source = undefined;
child.source = undefined;

leftChild.entries.push(this.entries[childIndex - 1]);
leftChild.size++;
this.entries[childIndex - 1] = child.entries.shift()!;
Expand All @@ -995,6 +1011,8 @@ export abstract class SortedBuilder<E> {
return;
}

rightChild.source = undefined;
child.source = undefined;
rightChild.entries.unshift(this.entries[childIndex]);
rightChild.size++;
this.entries[childIndex] = child.entries.pop()!;
Expand Down
18 changes: 18 additions & 0 deletions packages/sorted/src/common/base.mts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,8 @@ export abstract class SortedBuilder<E> {
}
}

// check whether child at given `childIndex` has gotten too small
// get from neighbouring children if it is too small so child increases
normalizeChildIncrease(childIndex: number): void {
const child = this.children[childIndex];
if (child.entries.length >= this.context.minEntries) return;
Expand All @@ -869,6 +871,7 @@ export abstract class SortedBuilder<E> {
// cannot shift
if (undefined !== leftChild) {
// join left
leftChild.source = undefined;
const [down] = this.entries.splice(childIndex - 1, 1);
leftChild.entries.push(down);
leftChild.entries = leftChild.entries.concat(child.entries);
Expand All @@ -882,6 +885,8 @@ export abstract class SortedBuilder<E> {
}

// join right
rightChild.source = undefined;
child.source = undefined;
const [down] = this.entries.splice(childIndex, 1);
rightChild.entries.unshift(down);
rightChild.entries = child.entries.concat(rightChild.entries);
Expand All @@ -900,6 +905,8 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length < leftChild.entries.length)
) {
// get from left
child.source = undefined;
leftChild.source = undefined;
child.entries.unshift(this.entries[childIndex - 1]);
child.size++;
this.entries[childIndex - 1] = leftChild.entries.pop()!;
Expand All @@ -916,6 +923,8 @@ export abstract class SortedBuilder<E> {
}

// get from right
child.source = undefined;
rightChild.source = undefined;
child.entries.push(this.entries[childIndex]);
child.size++;
this.entries[childIndex] = rightChild.entries.shift()!;
Expand All @@ -929,8 +938,11 @@ export abstract class SortedBuilder<E> {
}
}

// check whether child at given `childIndex` has gotten too large
// shift to neighbouring children if it is too large so child decreases
normalizeChildDecrease(childIndex: number): void {
const child = this.children[childIndex];

if (child.entries.length <= this.context.maxEntries) return;

const leftChild = this.children[childIndex - 1];
Expand All @@ -943,6 +955,7 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length >= this.context.maxEntries)
) {
// need to split child
child.source = undefined;
const index = (child.entries.length >>> 1) + 1;
const preSize = child.size;
const rightEntries = child.entries.splice(index);
Expand Down Expand Up @@ -980,6 +993,9 @@ export abstract class SortedBuilder<E> {
rightChild.entries.length >= leftChild.entries.length)
) {
// shiftleft
leftChild.source = undefined;
child.source = undefined;

leftChild.entries.push(this.entries[childIndex - 1]);
leftChild.size++;
this.entries[childIndex - 1] = child.entries.shift()!;
Expand All @@ -995,6 +1011,8 @@ export abstract class SortedBuilder<E> {
return;
}

rightChild.source = undefined;
child.source = undefined;
rightChild.entries.unshift(this.entries[childIndex]);
rightChild.size++;
this.entries[childIndex] = child.entries.pop()!;
Expand Down
26 changes: 26 additions & 0 deletions packages/sorted/test/sortedmap-issues.test.mts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Stream, Tuple } from '@rimbu/core';
import { SortedMap } from '../src/main/index.mjs';

describe('SortedMap issues fixed by PRs', () => {
it('from SortedSet issue #189: remove should not use reference equality', () => {
const set1 = SortedMap.of([Tuple.of(1, 'a'), 'q'], [Tuple.of(2, 'b'), 'v']);

expect(set1.getAtIndex(0)).toEqual([[1, 'a'], 'q']);
expect(set1.hasKey([1, 'a'])).toBe(true);

const s2 = set1.removeKey([1, 'a']);
expect(s2.size).toBe(1);
});

it('from SortedSet issue #188: `getAtIndex` causes TypeError', () => {
let map = SortedMap.from<number, number>([]);

for (const i of Stream.range({ start: 0, end: 5 })) {
// add [0 .. 10], [10 .. 20], [20 .. 30] ...
map = map.addEntries(
Stream.range({ start: i * 10, amount: 10 }).map((v) => [v, -v])
);
expect(map.getAtIndex(-1)).toEqual([i * 10 + 9, -(i * 10 + 9)]);
}
});
});
12 changes: 11 additions & 1 deletion packages/sorted/test/sortedset-issues.test.mts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SortedSet } from '../src/main/index.mjs';
import { Tuple } from '@rimbu/core';
import { Stream, Tuple } from '@rimbu/core';

describe('SortedSet issues fixed by PRs', () => {
it('issue #189: remove should not use reference equality', () => {
Expand All @@ -11,4 +11,14 @@ describe('SortedSet issues fixed by PRs', () => {
const s2 = set1.remove([1, 'a']);
expect(s2.size).toBe(1);
});

it('issue #188: `getAtIndex` causes TypeError', () => {
let set = SortedSet.from<number>([]);

for (const i of Stream.range({ start: 0, end: 5 })) {
// add [0 .. 10], [10 .. 20], [20 .. 30] ...
set = set.addAll(Stream.range({ start: i * 10, amount: 10 }));
expect(set.getAtIndex(-1)).toBe(i * 10 + 9);
}
});
});

0 comments on commit 3e19056

Please sign in to comment.