levinsonmm / MI-449-SS17-741-js-intro-Vh2K33

0 stars 0 forks source link

Project Feedback #1

Closed levinsonmm closed 7 years ago

levinsonmm commented 7 years ago

@chrisvfritz Can you take a look at this? It's hosted here and meets the following criteria:

chrisvfritz commented 7 years ago

Very nice! 🌟 I try hard to find some room for improvement, but you're making it pretty difficult this time. 😅 There are some tiny style improvements that could be made, but you'll learn about that in the next lesson. 🎉 :shipit:

Instead, I'll give you a taste of the real power of functions. 😄 Across the code to set up each button, you've maintained a very consistent strategy, so the only real difference between each is the name of the sound. That means we could actually simplify your code to:

var attachSound = function (soundName) {
  var sound = document.getElementById(soundName + 'Sound')
  var button = document.getElementById(soundName)

  var playSound = function () {
    sound.play()
  }

  button.addEventListener('click', playSound)
  button.addEventListener('mouseenter', playSound)
}

attachSound('clap')
attachSound('hihat')
attachSound('kick')

The attachSound function is provided the name of the sound, then we use that name to:

  1. Get the appropriate sound and button from the page
  2. Create a function to play that sound
  3. Set up the button to play the sound on click and mouseenter

If you have any questions/comments/etc about this or anything else, please don't hesitate to let me know! 😸 🎈

levinsonmm commented 7 years ago

Thanks for the feedback! Being more efficient is definitely important to me, so thank you for pointing out a better way to do this. 😄