Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(common/models): convert traversal.children method to return array 📖 #12089

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions common/models/templates/src/trie.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,12 @@ export class TrieTraversal implements LexiconTraversal {
}
}

*children(): Generator<{char: USVString, traversal: () => LexiconTraversal}> {
children(): {char: USVString, p: number, traversal: () => LexiconTraversal}[] {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably also faster, if we are not leveraging generator for partial results!

let root = this.root;
const totalWeight = this.totalWeight;

const results: {char: USVString, p: number, traversal: () => LexiconTraversal}[] = [];

// Sorts _just_ the current level, and only if needed.
// We only care about sorting parts that we're actually accessing.
sortNode(root, true);
Expand All @@ -183,21 +185,26 @@ export class TrieTraversal implements LexiconTraversal {
let internalNode = entryNode;
for(let lowSurrogate of internalNode.values) {
let prefix = this.prefix + entry + lowSurrogate;
yield {
results.push({
char: entry + lowSurrogate,
p: internalNode.children[lowSurrogate].weight / totalWeight,
traversal: function() { return new TrieTraversal(internalNode.children[lowSurrogate], prefix, totalWeight) }
}
});
}
} else {
// Determine how much of the 'leaf' entry has no Trie nodes, emulate them.
// No deduplication needed. If there were multiple keys available,
// we wouldn't be in a leaf node.
let fullText = entryNode.entries[0].key;
entry = entry + fullText[this.prefix.length + 1]; // The other half of the non-BMP char.
let prefix = this.prefix + entry;

yield {
const weight = entryNode.entries.reduce((best, current) => Math.max(best, current.weight), 0);

results.push({
char: entry,
p: weight / totalWeight,
traversal: function () {return new TrieTraversal(entryNode, prefix, totalWeight)}
}
});
}
} else if(isSentinel(entry)) {
continue;
Expand All @@ -206,34 +213,41 @@ export class TrieTraversal implements LexiconTraversal {
continue;
} else {
let prefix = this.prefix + entry;
yield {
results.push({
char: entry,
p: entryNode.weight / totalWeight,
traversal: function() { return new TrieTraversal(entryNode, prefix, totalWeight)}
}
});
}
}

return;
return results;
} else { // type == 'leaf'
let prefix = this.prefix;

// No actual deduplication needed. If there were multiple keys available,
// we wouldn't be in a leaf node.
let children = root.entries.filter(function(entry) {
return entry.key != prefix && prefix.length < entry.key.length;
})
});

let weight = children.reduce((best, current) => Math.max(current.weight, best), 0);

for(let {key} of children) {
if(children.length > 0) {
const key = children[0].content;
let nodeKey = key[prefix.length];

if(isHighSurrogate(nodeKey)) {
// Merge the other half of an SMP char in!
nodeKey = nodeKey + key[prefix.length+1];
}
yield {
results.push({
char: nodeKey,
p: weight / totalWeight,
traversal: function() { return new TrieTraversal(root, prefix + nodeKey, totalWeight)}
}
});
};
return;
return results;
}
}

Expand Down
19 changes: 6 additions & 13 deletions common/models/templates/test/test-trie-traversal.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,13 @@ describe('Trie traversal abstractions', function() {
do {
assert.isNotEmpty(leafChildSequence);

let iter = curChild.traversal().children();
let curr = iter.next();
curChild = curr.value;
let children = curChild.traversal().children();
assert.equal(children.length, 1);
curChild = children[0];

// Test generator behavior - there should be one child, then the 'done' state.
assert.isDefined(curChild);
assert.equal(curChild.char, leafChildSequence[0]);
curr = iter.next();
assert.isTrue(curr.done);

// Prepare for iteration.
leafChildSequence.shift();
Expand Down Expand Up @@ -275,15 +273,13 @@ describe('Trie traversal abstractions', function() {
do {
assert.isNotEmpty(leafChildSequence);

let iter = curChild.traversal().children();
let curr = iter.next();
curChild = curr.value;
let children = curChild.traversal().children();
assert.equal(children.length, 1);
curChild = children[0];

// Test generator behavior - there should be one child, then the 'done' state.
assert.isDefined(curChild);
assert.equal(curChild.char, leafChildSequence[0]);
curr = iter.next();
assert.isTrue(curr.done);

// Prepare for iteration.
leafChildSequence.shift();
Expand Down Expand Up @@ -337,9 +333,6 @@ describe('Trie traversal abstractions', function() {
// Just to be sure our utility function is working right.
assert.equal(smpA + smpP + 'pl' + smpE, "𝖺𝗉pl𝖾");

let pKeys = ['p', smpP];
let leafChildSequence = ['l', smpE];

const aNode = rootTraversal.child(smpA);
assert.isOk(aNode);
assert.isNotOk(rootTraversal.child('a'));
Expand Down
2 changes: 1 addition & 1 deletion common/models/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ declare interface LexiconTraversal {
* - If `char` = 'e', the child represents a prefix of 'the'.
* - Then `traversal` allows traversing the part of the lexicon prefixed by 'the'.
*/
children(): Generator<{char: USVString, traversal: () => LexiconTraversal}>;
children(): {char: USVString, p: number, traversal: () => LexiconTraversal}[];

/**
* Allows direct access to the traversal state that results when appending one
Expand Down
Loading