How can I shorten my code with for() or something else in JavaScript?

Question

WHAT I WANT TO DO

I want to shorten my code. This Drum Play App plays sound by pressing certain keys or clicking with your mouse.

It works, but the code for click events is too long because I repeated the same code many times.

Could anyone make it cleaner/shorter?

WHAT I TRIED

I tried for loop, like below:

document.querySelector(`div[data-key="${dataKeys[i]}"]`).addEventListener...

But it didn't work.

MY CURRENT CODE

Here is my code.

<body>

  <div class="keys">
    <div data-key="65" class="key">
      <kbd>A</kbd>
      <span class="sound">clap</span>
    </div>
    <div data-key="83" class="key">
      <kbd>S</kbd>
      <span class="sound">hihat</span>
    </div>
    <div data-key="68" class="key">
      <kbd>D</kbd>
      <span class="sound">kick</span>
    </div>
    ...

  </div>

  <audio data-key="65" src="sounds/clap.wav"></audio>
  <audio data-key="83" src="sounds/hihat.wav"></audio>
  <audio data-key="68" src="sounds/kick.wav"></audio>
  ...

<script>
  window.addEventListener('keydown', function(e) {
    const audio = document.querySelector(`audio[data-key="${e.keyCode}"]`);
    if (!audio) return; // Stop the function from running all together
    audio.currentTime = 0; // Rewind to the start
    audio.play();
  });

  document.querySelector('div[data-key="65"]').addEventListener('click', function(e) {
    console.log('I clicked.')
    const clickedAudio = document.querySelector(`audio[data-key="65"]`);
    clickedAudio.currentTime = 0; // Rewind to the start
    clickedAudio.play();
  });

  document.querySelector('div[data-key="83"]').addEventListener('click', function(e) {
    console.log('I clicked.')
    const clickedAudio = document.querySelector(`audio[data-key="83"]`);
    clickedAudio.currentTime = 0; // Rewind to the start
    clickedAudio.play();
  });

  document.querySelector('div[data-key="68"]').addEventListener('click', function(e) {
    console.log('I clicked.')
    const clickedAudio = document.querySelector(`audio[data-key="68"]`);
    clickedAudio.currentTime = 0; // Rewind to the start
    clickedAudio.play();
  });
  ...

</script>

</body>

I'd appreciate it if anyone could make my code cleaner or shorter.

You can add a common class to all the `divs` and access the clicked class using `this` variable — Krishna Prashatt, May 06 '19 at 10:19
The reason you can't attach listeners in a loop is due to closures. Check [this](https://stackoverflow.com/questions/19586137/addeventlistener-using-for-loop-and-passing-values) thread. — Nuhman, May 06 '19 at 10:20
Use `e.getAttribute( 'data-key' );` the same way you use `e.keyCode`. That way you can have one click function for all the divs. — Shilly, May 06 '19 at 10:20

score 6 · Accepted Answer · answered May 06 '19 at 10:18

6

Select all .key elements, check their dataset to get their associated key number, and then you can dynamically select the associated audio:

document.querySelectorAll('.key').forEach((div) => {
  div.addEventListener('click', () => {
    const { key } = div.dataset;
    const clickedAudio = document.querySelector(`audio[data-key="${key}"]`);
    clickedAudio.currentTime = 0; // Rewind to the start
    clickedAudio.play();
  });
});

answered May 06 '19 at 10:18

CertainPerformance

356,069
52
309
320

Thank you very much for your answer. Your code works well! Let me ask one more question. What does this line mean? : const { key } = div.dataset; – Miu May 06 '19 at 12:01
1

That's destructuring. It extracts the `key` property of the `dataset` object and puts it into a variable named `key`. – CertainPerformance May 06 '19 at 12:24

How can I shorten my code with for() or something else in JavaScript?

1 Answers1