0

I have two input fields that each trigger a function when their content changes.

document.getElementsByName("playerOut")[0].addEventListener('change', swapPlayerOut);
document.getElementsByName("playerIn")[0].addEventListener('change', swapPlayerIn);

Each of those two functions dynamically update innerHTML content based on the input.

// Swaps out the player being sold
function swapPlayerOut() {
  var om = document.getElementsByClassName("outMil")[0];
  var result = entries.find(player => player.gsx$playerfullname.$t === this.value);
  om.innerHTML = result.gsx$value.$t;
  a = result.gsx$value.$t;
};

// Swaps in the player being bought
function swapPlayerIn() {
  var im = document.getElementsByClassName("inMil")[0];
  var result = entries.find(player => player.gsx$playerfullname.$t === this.value);
  im.innerHTML = result.gsx$value.$t;
  b = result.gsx$value.$t;
};

And the HTML:

<div class="grid-container-transfers">
  <div class="outMil"></div>
  <div class="inMil"></div>
  <div class="PandL"></div>
</div>

I need to update the third HTML element <div class="PandL"> with a calculation of a and b that are being passed outside of their local functions.

I've got it working with two more event listeners:

document.getElementsByName("playerOut")[0].addEventListener('change', pl);
document.getElementsByName("playerIn")[0].addEventListener('change', pl2);

calling two more functions:

function pl () {
    var pandl = document.getElementsByClassName("PandL")[0];
    pandl.innerHTML = -a;
    }

function pl2 () {
    var pandl = document.getElementsByClassName("PandL")[0];
    var round = a-b;
    pandl.innerHTML = round.toFixed(1);
        }

But this feels MIGHTY ineffecient and unstable, so I'm wondering if anyone knows of a more efficient way of achieving the calculation of a-b.

I'm still quite new to js, and I'm sure this is 101 stuff, but if anyone knows how to do this, I'd be grateful. Cheers!

Jimmy
  • 29
  • 5
  • 1
    The most important thing you should immediately do to improve performance is: #1 Get your DOM references outside of your functions so you don't wind up scanning for the same elements over and over again and #2 [**Stop using `getElementsByName` and `getElementsByClassName` and definitely don't throw an index on to the results of those methods.](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474)** – Scott Marcus Aug 24 '20 at 21:26
  • As for the rest, you don't need separate `swapPlayerOut` and `swapPlayerIn` functions that do exactly the same thing, but just with a different element reference. Instead, just make one `swapPlayer` function that takes an argument which will be the element that needs to be swapped. Same idea with `pl` and `pl2`. – Scott Marcus Aug 24 '20 at 21:28
  • And also, don't use `.innerHTML` when the strings you are working with don't contain any HTML as `.innerHTML` has security and performance implications. – Scott Marcus Aug 24 '20 at 21:42
  • Try out jquery - www.jquery.com – koder613 Aug 24 '20 at 21:49
  • @koder613 For the love of God, please don't advocate JQuery for simple DOM operations. Especially, when someone doesn't have a strong understanding of vanilla JavaScript. – Scott Marcus Aug 24 '20 at 21:58
  • Thanks @ScottMarcus – all good recommendations! Not yet up to snuff on best practise, but this helps a lot. – Jimmy Aug 25 '20 at 18:15

0 Answers0