2
\$\begingroup\$

This is just a simple little drum machine. Since it loads sounds files, I can't just put it on JSFiddle. I tried Github pages too, but am not sure how to get the sound files working. However, you can clone the repo, run a server with python -m SimpleHTTPServer 8000 (it won't work if opened with the file:// protocol), and open index.html.

Hopefully you can click on the buttons and see the drum machine going. However, it's not very performant. Setting even half the buttons to "on" at the same time will cause some sounds to skip. Not having a lot of experience with optimization, I could use a look at the code.

$(function(){
  // these sound files actually have aif extension
  sound_files = [
    "./hihat_closed2",
    "./909-klick",
    "./909-dist",
  ]
  sounds = []
  tracks = []
  sound_files.forEach(function(file){
    sounds.push(
      // use buzz libary to create sound object
      new buzz.sound(file, { formats: ["mp3"] })
    )
  })
  // 3 iterations because there are 3 sounds
  $.each(new Array(3), function(idx, nothing){
    tracks = tracks // holds the beats for a single sound
    tracks.push(new Array(16)) // the 16 beats
    var $triggers = $("[sound='"+idx+"']") // all a sound's buttons
    idx = idx
    $triggers.on("click", function(e){ // when a button is clicked
      var $e = $(e.currentTarget)
      var beat = parseInt($e.attr("beat")) // find the beat idx
      if (tracks[idx][beat] === 1){ 
        tracks[idx][beat] = 0 // toggle the beat on / off
      } else { tracks[idx][beat] = 1 }
    }.bind(this))
  }.bind(this))

  window.setInterval(
    function(){
      // play the 16 beats
      $.each(new Array(16), function(beat_idx, nothing){
        [0,1,2].forEach(function(sound_idx){
          // play a sound using setTimeout
          // delay the sound being played by n milliseconds,
          // where n depends on the beat idx
          window.setTimeout(function(){
            if (tracks[sound_idx][beat_idx] === 1) {
              var $trigger = $("[sound='"+sound_idx+"'][beat='"+beat_idx+"']")
              // when the beat is played, add CSS to its button
              $trigger.addClass("playing")
              sounds[sound_idx].play()
            }
            console.log("playign sound")
          }, (200 * (beat_idx + 1)))
        })
      })
      console.log("nwe loop")
      // remove the style for playing buttons until next loop
      // creates them again
      $(".playing").removeClass("playing")
    }, 3200
  )

The DOM it is working with is like as follows:

<head>
<script src="https://rawgit.com/jaysalvat/buzz/master/dist/buzz.js"></script>
<script src="https://code.jquery.com/jquery-git2.min.js"></script>
<style>
  .playing {
    border: 1px solid red;
  }
</style>
</head>
<body>

<h3> Sound 1</h3>
<button sound="0" beat="0">0</button>
<button sound="0" beat="1">1</button>
<button sound="0" beat="2">2</button>
<button sound="0" beat="3">3</button>
<button sound="0" beat="4">4</button>
<button sound="0" beat="5">5</button>
<button sound="0" beat="6">6</button>
<button sound="0" beat="7">7</button>
<button sound="0" beat="8">8</button>
<button sound="0" beat="9">9</button>
<button sound="0" beat="10">10</button>
<button sound="0" beat="11">11</button>
<button sound="0" beat="12">12</button>
<button sound="0" beat="13">13</button>
<button sound="0" beat="14">14</button>
<button sound="0" beat="15">15</button>

<h3> Sound 2</h3>

<button sound="1" beat="0">0</button>
<button sound="1" beat="1">1</button>
<button sound="1" beat="2">2</button>
<button sound="1" beat="3">3</button>
<button sound="1" beat="4">4</button>
<button sound="1" beat="5">5</button>
<button sound="1" beat="6">6</button>
<button sound="1" beat="7">7</button>
<button sound="1" beat="8">8</button>
<button sound="1" beat="9">9</button>
<button sound="1" beat="10">10</button>
<button sound="1" beat="11">11</button>
<button sound="1" beat="12">12</button>
<button sound="1" beat="13">13</button>
<button sound="1" beat="14">14</button>
<button sound="1" beat="15">15</button>


<h3> Sound 3</h3>
<button sound="2" beat="0">0</button>
<button sound="2" beat="1">1</button>
<button sound="2" beat="2">2</button>
<button sound="2" beat="3">3</button>
<button sound="2" beat="4">4</button>
<button sound="2" beat="5">5</button>
<button sound="2" beat="6">6</button>
<button sound="2" beat="7">7</button>
<button sound="2" beat="8">8</button>
<button sound="2" beat="9">9</button>
<button sound="2" beat="10">10</button>
<button sound="2" beat="11">11</button>
<button sound="2" beat="12">12</button>
<button sound="2" beat="13">13</button>
<button sound="2" beat="14">14</button>
<button sound="2" beat="15">15</button>
\$\endgroup\$
2
\$\begingroup\$

Missing syntax:

In JavaScript, while the engine does not need it to run, you're supposed to have semi-colons ending every line:

  sound_files = [
    "./hihat_closed2",
    "./909-klick",
    "./909-dist",
  ]
  sounds = []
  tracks = []

Iteration:

$.each(new Array(3)
[0,1,2].forEach

This is not how we do iteration. Use a for loop instead.


jQuery [Ab|Mis]use:

Every single feature you use in jQuery is easily usable in JavaScript:

  • $.each: forEach
  • $("[sound='"+idx+"']"): document.QuerySelectorAll, read my below point about template strings too.
  • $(function(){: document.onload or document.addEventListener('load', ...

parseInt:

You should be using the second parameter when using parseInt, because you need to provide the numerical system you're trying to convert into.

If you're using base-10 / decimal, you should use Number instead, as it is more clear and without unknowing side-effects:

parseInt($e.attr("beat"))

into:

Number($e.attr("beat"))

Magic Numbers:

  • new Array(3): What is 3?
  • new Array(16): What is 16?
  • [0,1,2]: What is 3?
  • 3200: What is 3200?
  • (200 * (beat_idx + 1)): What is 200 times beat_idx plus one?

Convert these to actual variable (with relevant names) so that maintainers know what each of these actually do.


Pointless things:

tracks = tracks // holds the beats for a single sound

In that line, you're assigning something to itself. that's literally doing nothing.


Template strings:

Provided that your system allows for ES6, you can use the magical template strings to build your content:

"[sound='"+sound_idx+"'][beat='"+beat_idx+"']"

into:

`[sound='${sound_idx}'][beat='${beat_idx}']`

Incorrekt spelling:

  • console.log("nwe loop")
  • console.log("playign sound")
\$\endgroup\$
  • \$\begingroup\$ thanks, ill try and implement these fixes. also lmao about the Pointless Things section but what I was actually trying to declare there is declare a variable from the function argument so that it could be passed to callbacks via bind. It's been a while since i wrote OO javascript, forgive me. \$\endgroup\$ – maxple Jan 29 '16 at 4:58
  • \$\begingroup\$ Why are you discouraging use of foreach? \$\endgroup\$ – Carcigenicate Jan 31 '16 at 0:57
  • \$\begingroup\$ @Carcigenicate I'm not discouraging the use of forEach, I'm discouraging making an temporary array to iterate three times instead of using a for loop. \$\endgroup\$ – Quill Jan 31 '16 at 0:58

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.