A shorter way to write for loops avoiding making too many similar if's?

Question

I would like to avoid writing lots of code for a big list of items. For example, I have this:

let f_a = document.querySelector('#FToneA');
let f_b = document.querySelector('#FToneB');
let f_c = document.querySelector('#FToneC');
// etc...
let fNameList = [f_a, f_b, f_c,]; // etc...

let fToneA = new Audio('../instrumentarium/assets/snd/flute/A_F.wav');
let fToneB = new Audio('../instrumentarium/assets/snd/flute/B_F.wav');
let fToneC = new Audio('../instrumentarium/assets/snd/flute/C_F.wav');
// etc...
let fToneList = [fToneA, fToneB, fToneC,]; // etc...

And then I have the events that should happen with each of the elements, that is a .wav file plays when you mouse over an element.

for (var t of fNameList) {
    t.addEventListener('mouseover', function() {
        function tSet(tone) {
            tone.play();
        };
    if (this === f_a) {
        tSet(fToneA);
    } else if (this === f_b) {
        tSet(fToneB);
    } else if (this === f_c) {
        tSet(fToneC);
    } // etc...

And what I want to do is similar to what you would do in Python with list.index(), so what I tried doing instead of the above is the following:

for (var nameIndex of fNameList) {
    nameIndex.addEventListener('mouseover', function() {
        for (var toneIndex of fToneList) {
            if (fNameList.indexOf(nameIndex) === fToneList.indexOf(toneIndex)) {
                toneIndex.play();
            }
        }
    })
};

In other words, in Python it would be just index() instead of indexOf(). But I suppose I'm doing something wrong here. Most likely at the line above "toneIndex.play()". Because what happens is only one sound plays for every element I hover/click.

Can anyone help me in writing this properly?

You don't need a loop to get the index. Just put `const index = fNameList.indexOf(this); if (index != -1) fToneList[index].play()` in your event handler. — Bergi, Nov 26 '19 at 23:44

score 5 · Accepted Answer · answered Nov 26 '19 at 23:38

5

Consider using an array of arrays, pairing each element (eg, #FToneA) to the associated audio, and then referencing both in the closure:

const audioTonePairs = [
  [document.querySelector('#FToneA'), new Audio('../instrumentarium/assets/snd/flute/A_F.wav')],
  [document.querySelector('#FToneB'), new Audio('../instrumentarium/assets/snd/flute/B_F.wav')],
  [document.querySelector('#FToneC'), new Audio('../instrumentarium/assets/snd/flute/C_F.wav')]
];

for (const [toneElm, audio] of audioTonePairs) {
  toneElm.addEventListener('mouseover', () => audio.play());
}

Make sure to use const, not var, so that the loop closure works properly.

You might want to construct the audioTonePairs less repetitively too:

const audioTonePairs = ['A', 'B', 'C'].map(key => [
  document.querySelector(`#FTone${key}`),
  new Audio(`../instrumentarium/assets/snd/flute/${key}_F.wav`),
]);

answered Nov 26 '19 at 23:38

CertainPerformance

356,069
52
309
320

`for of`, that's a new one. – Jared Farrish Nov 26 '19 at 23:43
I wouldn't be able to come up with such a short solution, thanks! In fact, I found out that in my case I only needed to replace "nameIndex" in the line above toneIndex.play() with "this". And it worked too! But thanks a lot, this solution is even better! – Mikhail Romantsov Nov 26 '19 at 23:47
Also, I didn't know a method with ${key}, this is going to make my life a whole lot easier from now on =) – Mikhail Romantsov Nov 26 '19 at 23:53
@JaredFarrish there is `for of` and `for in`, the difference is `of` follows an order (0, 1, 2, 3, N) and `in` does it randomly, without a specific order, for all I know. – Mikhail Romantsov Nov 26 '19 at 23:54
@MikhailRomantsov `for in` iterates over the property names of the object (and can be done for any object), in [deterministic order](https://stackoverflow.com/questions/30076219/does-es6-introduce-a-well-defined-order-of-enumeration-for-object-properties/58444453#58444453). `for of`, on the other hand, only works with objects which have *iterators*, like arrays and other indexed collections (and iterates in the order that the iterator returns elements). – CertainPerformance Nov 26 '19 at 23:56

Mikhail Romantsov · Answer 2 · 2019-11-27T00:08:21.753

Found a solution to my example. I only had to use this instead of nameIndex:

for (var nameIndex of fNameList) {
    nameIndex.addEventListener('mouseover', function() {
        for (var toneIndex of fToneList) {
            if (fNameList.indexOf(this) === fToneList.indexOf(toneIndex)) {
                toneIndex.play();
            }
        }
    })
};

I still would want to make it shorter, though. If possible.

A shorter way to write for loops avoiding making too many similar if's?

2 Answers2