3
\$\begingroup\$

This small piece of code shows the value of different fields of an array. I'm looking for a way to improve this nested ternary function. An if statement solution would be obvious, so I'm wondering if there is something more elegant to solve it.

<button type="button" onclick="myFunction()">Check</button>
<p id="demo"></p>

<script>
function getArray(value){
    var showValue = (value.value1 > 0) ? value.value1 :
                        (value.value2 > 0) ? value.value2 :
                            (value.value3 > 0) ? value.value3 : 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},
  ];
  
  array.map(getArray);

}
</script>

Output

Abba: 1 Mars: 4 Nexus: 6
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.