I'm looking for a way to improve this nested ternary function
They keys to check could be iterated over - e.g.
function getArray(value) {
var showValue = 0;
for (let key of ['value1', 'value2', 'value3']) {
if (value[key] > 0) {
showValue = value[key];
break;
}
}
document.getElementById("demo").innerHTML += value.system + ": " + showValue + " ";
}
function myFunction() {
var array = [
{system:"Abba", value1:0, value2:1, value3:0},
{system:"Mars", value1:0, value2:4, value3:0},
{system:"Nexus", value1:0, value2:0, value3:6},
{system:"Zeroes", value1:0, value2:0, value3:0}
];
array.forEach(getArray);
}
<button type="button" onclick="myFunction()">Check</button>
<p id="demo"></p>
And this could be simplified using the array method find()
combined with logical OR - i.e. ||
for a fall-back value using Short-circuit evaluation for the case when no values are greater than zero:
function getArray(value){
//find the key that has a value greater than zero
const key = ['value1', 'value2', 'value3'].find(key => value[key] > 0);
//return the value that is greater than zero, or else zero in case no value is greater than zero
const showValue = value[key] || 0;
document.getElementById("demo").innerHTML += value.system + ": "+ showValue + " ";
}
function myFunction() {
var array = [
{system:"Abba", value1:0, value2:1, value3:0},
{system:"Mars", value1:0, value2:4, value3:0},
{system:"Nexus", value1:0, value2:0, value3:6},
{system:"Zeroes", value1:0, value2:0, value3:0}
];
array.forEach(getArray);
}
<button type="button" onclick="myFunction()">Check</button>
<p id="demo"></p>
Other review points
Improper use of map
method
The map()
method "creates a new array populated with the results of calling a provided function on every element in the calling array"1. For this code forEach()
can be used (as it is in the two snippets above) since the return value of each iteration is not used after the callback is executed. .map()
could be used to return a string for each object and then update the DOM element only once, which would be better for multiple reasons:
- fewer DOM lookups - remember those aren't cheap
Array.join()
can be used to separate each string with a space, without adding an excess space at the end of the output
- the function would only have one responsibility - in-line with the Single Responsibility principle
function getArray(value){
const key = ['value1', 'value2', 'value3'].find(key => value[key] > 0);
const showValue = value[key] || 0;
return value.system + ": "+ showValue;
}
function myFunction() {
var array = [
{system:"Abba", value1:0, value2:1, value3:0},
{system:"Mars", value1:0, value2:4, value3:0},
{system:"Nexus", value1:0, value2:0, value3:6},
{system:"Zeroes", value1:0, value2:0, value3:0}
];
document.getElementById("demo").innerHTML = array.map(getArray).join(" ");
}
<button type="button" onclick="myFunction()">Check</button>
<p id="demo"></p>
Keep HTML and JavaScript separate
The HTML has an onclick
attribute:
<button type="button" onclick="myFunction()">Check</button>
Instead of using that attribute, the event handling can be setup in Javascript - e.g. using document.addEventListener()
:
document.getElementById('checkBtn').addEventListener('click', myFunction);
this would require an id attribute be added to the element - e.g.
<button type="button" id="checkBtn">Check</button>
though other ways to detect it are possible (e.g. class name, elements of document.forms
, etc. With this separation the JavaScript (logic) can be modified without ever needing to alter the HTML (markup) - especially helpful in larger projects where one person might update HTML and another the JS.