Skip to content

Commit

Permalink
Merge pull request #133 from phyloref/remove-unguarded-external-speci…
Browse files Browse the repository at this point in the history
…fiers

While working on Klados, I discovered that some uses of `phyloref.internalSpecifiers`/`phyloref.externalSpecifiers` are not guarded: we access those variables without checking to see if they exist. Generally this causes an error that is simply ignored. But since it is an easy fix, I went ahead and fixed them in this PR.
  • Loading branch information
gaurav authored Aug 2, 2023
2 parents ec8c86b + 2a1f956 commit e851049
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/wrappers/PhylorefWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class PhylorefWrapper {
/** Return the internal specifiers of this phyloref (if any). */
get internalSpecifiers() {
if (!has(this.phyloref, 'internalSpecifiers')) {
// If there isn't one, create an empty list so that the caller can do
// `wrappedPhyloref.internalSpecifiers.push({...})`.
this.phyloref.internalSpecifiers = [];
}

Expand All @@ -34,6 +36,8 @@ class PhylorefWrapper {
/** Return the external specifiers of this phyloref (if any). */
get externalSpecifiers() {
if (!has(this.phyloref, 'externalSpecifiers')) {
// If there isn't one, create an empty list so that the caller can do
// `wrappedPhyloref.externalSpecifiers.push({...})`.
this.phyloref.externalSpecifiers = [];
}

Expand Down Expand Up @@ -114,11 +118,15 @@ class PhylorefWrapper {
// it doesn't remember if the specifier to be deleted is internal
// or external. We delete the intended specifier from both arrays.

let index = this.phyloref.internalSpecifiers.indexOf(specifier);
if (index !== -1) this.phyloref.internalSpecifiers.splice(index, 1);
if (has(this.phyloref, 'internalSpecifiers') && this.phyloref.internalSpecifiers.length > 0) {
const index = this.phyloref.internalSpecifiers.indexOf(specifier);
if (index !== -1) this.phyloref.internalSpecifiers.splice(index, 1);
}

index = this.phyloref.externalSpecifiers.indexOf(specifier);
if (index !== -1) this.phyloref.externalSpecifiers.splice(index, 1);
if (has(this.phyloref, 'externalSpecifiers') && this.phyloref.externalSpecifiers.length > 0) {
const index = this.phyloref.externalSpecifiers.indexOf(specifier);
if (index !== -1) this.phyloref.externalSpecifiers.splice(index, 1);
}
}

getExpectedNodeLabels(phylogeny) {
Expand Down

0 comments on commit e851049

Please sign in to comment.