Skip to content

Commit

Permalink
MNT: add parseAndValidateLineWidth() and tests
Browse files Browse the repository at this point in the history
Per work on biocore#135 / biocore#144.
  • Loading branch information
fedarko committed Jul 17, 2020
1 parent 406849c commit b774d0a
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 8 deletions.
63 changes: 55 additions & 8 deletions empress/support_files/js/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,12 @@ define(["underscore"], function (_) {
// separate the numeric and the alpha elements of the array
// Note that NaN and +/- Infinity are not considered numeric elements
for (var index = 0; index < list.length; index++) {
if (
isNaN(parseFloat(list[index])) ||
!isFinite(parseFloat(list[index]))
) {
alphaPart.push(list[index]);
var origVal = list[index];
var pfVal = parseFloat(origVal);
if (isNaN(pfVal) || !isFinite(pfVal)) {
alphaPart.push(origVal);
} else {
numericPart.push(list[index]);
numericPart.push(origVal);
}
}

Expand All @@ -100,6 +99,24 @@ define(["underscore"], function (_) {
return result.concat(alphaPart, numericPart);
}

/**
* Returns true if a string represents a finite number, false otherwise.
*
* Note that number parsing / type conversions in general are extremely
* hairy in JS. This function seems to work well for a wide range of cases,
* but it may not be perfect.
*
* This function was created from code that was originally in
* splitNumericValues() in Emperor's codebase. That code, in turn, was
* based on https://stackoverflow.com/a/9716488.
*
* @param {String} value Value to check for validity
* @return {Boolean} true if value represents a finite number, else false
*/
function isValidNumber(value) {
return (!isNaN(parseFloat(value)) && isFinite(value));
}

/**
* Split list of string values into numeric and non-numeric values
*
Expand All @@ -115,8 +132,7 @@ define(["underscore"], function (_) {
var numeric = [];
var nonNumeric = [];
_.each(values, function (element) {
// https://stackoverflow.com/a/9716488
if (!isNaN(parseFloat(element)) && isFinite(element)) {
if (isValidNumber(element)) {
numeric.push(element);
} else {
nonNumeric.push(element);
Expand All @@ -143,10 +159,41 @@ define(["underscore"], function (_) {
}, effectiveDuration);
}

/**
* Returns a numeric representation of a line width <input> element.
*
* If the value of the input is in some way "invalid" (i.e. isValidNumber()
* is false for this value, or the number is less than 0) then we will
* 1) set the value of the <input> to 0 ourselves, and
* 2) return 0.
*
* @param {HTMLElement} inputEle A reference to an <input> element
* describing a line width to use in coloring
* the tree. This should ideally have
* type="number" and min="0" set so that the
* user experience is consistent, but none of
* these things are checked for here -- we
* only look at the value of this element.
* @return {Number} Sanitized number that can be used as input to
* Empress.thickenSameSampleLines().
*/
function parseAndValidateLineWidth(inputEle) {
if (isValidNumber(inputEle.value)) {
var pfVal = parseFloat(inputEle.value);
if (pfVal >= 0) {
return parseFloat(inputEle.value);
}
}
// If we're still here, the number was invalid.
inputEle.value = 0;
return 0;
}

return {
keepUniqueKeys: keepUniqueKeys,
naturalSort: naturalSort,
splitNumericValues: splitNumericValues,
parseAndValidateLineWidth: parseAndValidateLineWidth,
toastMsg: toastMsg,
};
});
1 change: 1 addition & 0 deletions tests/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

<!-- add any test GUI components to this div-->
<div id="test-div"></div>
<input id="test-num-input" type="number">

<!-- The tree canvas need for use in test-empress-->
<div id="tree-container" class="{{ emperor_classes }}">
Expand Down
24 changes: 24 additions & 0 deletions tests/test-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,5 +253,29 @@ require(["jquery", "util"], function ($, util) {
deepEqual(resultArray, expectedArray);
}
});

test("Test parseAndValidateLineWidth (invalid case)", function() {
var tni = document.getElementById("test-num-input");
// force the test input's value to be -2
// (In practice, min="0" should prevent the values of Empress' line
// width inputs from being less than 0, but I don't really trust
// those to be perfect safeguards. Hence the paranoia.)
tni.value = "-2";
// Double-check that the value is -2 (so that we can verify that
// parseAndValidateLineWidth() actually *changed* this value)
deepEqual(tni.value, "-2");
var lw = util.parseAndValidateLineWidth(tni);
deepEqual(lw, 0);
deepEqual(tni.value, "0");
});

test("Test parseAndValidateLineWidth (valid case)", function() {
var tni = document.getElementById("test-num-input");
tni.value = "2.5";
deepEqual(tni.value, "2.5");
var lw = util.parseAndValidateLineWidth(tni);
deepEqual(lw, 2.5);
deepEqual(tni.value, "2.5");
});
});
});

0 comments on commit b774d0a

Please sign in to comment.