Video Player Clone Project

You can’t really expect people to get dropped into some code with no information about anything and come up with a solution. I’m not even sure what a working version would look like.


  1. videoPlayer.init is called with an object. You can add the array of video ids as a property.
videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  },
  videos: [
    "0dgNc5S8cLI",
    "mnfmQe8Mv1g",
    "CHahce95B1g",
    "2VwsvrPFr9w"
  ]
});
  1. As such the function must accept an object (options object). The argument to videoPlayer.init will be an object with properties which you can then access in the function.
function init(options) {
  addEvents(options.afterPlayerReady);
  config.playlist = options.videos.join();
  loadIframeScript();
  window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
}
  1. You have to change the parameter to addEvents it now is passed the one handler function (I renamed the parameter to handler).
function addEvents(handler) {
  eventHandlers.afterPlayerReady = handler;
  events.afterPlayerReady = new Event("afterPlayerReady");
}

Fork. I have no idea if it is working as intended because I have no idea what that is.

1 Like

@lasjorg

The code seems to work both ways.

Is one way more correct than the other?
or a better way of writing it?

It works, or seems to work the way it was originally written: Code 2

All I did was add this:

}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

It works like this: Code 1 https://jsfiddle.net/y729kuwc/

function addEvents(handler) {
    eventHandlers.afterPlayerReady = handler;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(options) {
    addEvents(options.afterPlayerReady);
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

And it works like this: Code 2 https://jsfiddle.net/8zm6gnwf/

How it was written originally.

function addEvents(handlers) {
    eventHandlers.afterPlayerReady = handlers.afterPlayerReady;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(initEventHandlers) {
    addEvents(initEventHandlers);
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

I don’t understand man. You have been working on this project, quite literally, for years.

If you had instead started by learning Javascript basics and then learned about the libraries and frameworks you are using here, you would have a far, far more functional app by this point.

1 Like

Spend some time to go through the freeCodeCamp curriculum, then come back to this project, it will make much more sense

The code works both ways, I just wanted which way is the preferred way.

I really doubt that the full application works the same either way. That would make no sense in this context.

You have made statements like this before only to find out that they are not true. A change like this has some effects. It literally says in the code that there is a difference.

Making some unit tests would really help you determine when your changes break your app.

In order to write unit tests, it would probably help if you learn the fundamentals of the languages, libraries, and frameworks that you are using.

Refactoring your code with unit tests and the fruits of learning foundational knowledge will also make it far easier for anyone to help you with your code.

hey bro can you look at my question, i am waiting for someone to check my qs

Please don’t spam other people’s posts. Thank you!

1 Like
  1. You are now passing two arguments to the function but the function only has one parameter. You are not using the array of ids you are passing to the function. The reason why it works is that you have added the ids to playlist in the addPlayer function. Passing it the array as the code is written now does nothing (which is just confusing and poor code). The fact that you are passing two arguments to a function with one parameter is pretty telling.

  2. How many functions you want to pass the object through before accessing the properties is just a matter of code organization. But the first version makes a lot more sense to me. The second is using incorrect semantics with plurals for no reason and what I would consider misleading naming.

1 Like

The same would go for this also?

Code 1 is better than Code 2?

Even without there being an array.

Code 1
https://jsfiddle.net/4fqcaust/

 function addEvents(handler) {
    eventHandlers.afterPlayerReady = handler;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(options) {
    addEvents(options.afterPlayerReady);
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
});

Code 2
https://jsfiddle.net/6t5pvsb4/

  function addEvents(handlers) {
    eventHandlers.afterPlayerReady = handlers.afterPlayerReady;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(initEventHandlers) {
    addEvents(initEventHandlers);
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
});

Better is an opinion, but yes I think it is. Although without the array it isn’t really an options object anymore. So the name options is misleading at that point.

  1. initEventHandlers sounds like a function/method that does something. Not an object containing a method. Passing it to a function makes it look like a callback, which it isn’t. There is also just one function/method so it wouldn’t be plural.
// initEventHandlers looks like a callback function
addEvents(initEventHandlers);
  1. Same with handlers, that would only make sense if the object contained multiple handler functions, otherwise it shouldn’t be plural.
1 Like

What name would you change options to instead?

options isn’t a terrible name, I should have called it “a bit” misleading.

It is still an object passed to an init method so calling it options still makes sense and allows for code extension. That is, you can add more properties to the object at a later date and it won’t look weird or break the API. I would probably just keep it as options.

1 Like

Code 3 is bad.

Between Code 1, and Code 2, which way is better?

Code 1 I have written a little differently, is that way good?

These are the main differences between the two.

Code 2 uses this:

config.playlist = options.videos.join();

  videos: [
    "0dgNc5S8cLI",
    "mnfmQe8Mv1g",
    "CHahce95B1g",
    "2VwsvrPFr9w"
  ]
});

Code 1 uses this:

config.playlist = videos.join();

}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

Code 1 https://jsfiddle.net/mfxht794/1/

  function addEvents(handler) {
    eventHandlers.afterPlayerReady = handler;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(options, videos) {
    addEvents(options.afterPlayerReady);
    config.playlist = videos.join();
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

Code 2 https://jsfiddle.net/948nkpto/

function addEvents(handler) {
    eventHandlers.afterPlayerReady = handler;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(options) {
    addEvents(options.afterPlayerReady);
    config.playlist = options.videos.join();
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  },

  videos: [
    "0dgNc5S8cLI",
    "mnfmQe8Mv1g",
    "CHahce95B1g",
    "2VwsvrPFr9w"
  ]
});

Code 3 https://jsfiddle.net/s4cn0f2b/1/

function addEvents(handlers) {
    eventHandlers.afterPlayerReady = handlers.afterPlayerReady;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(initEventHandlers, videos) {
    addEvents(initEventHandlers);
    config.playlist = videos.join();
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());

videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  }
}, [
  "0dgNc5S8cLI",
  "mnfmQe8Mv1g",
  "CHahce95B1g",
  "2VwsvrPFr9w"
]);

I prefer the second. Use the object you pass to the init function. The fewer parameters a function has the better. One of the reasons for using an options objects is to avoid having to pass the arguments in the correct order and it can also help make the API (the interface to the function) cleaner.

You will often see it when a function needs to accept a lot of arguments. Like fetch, for example. The second argument is an options object.

Source MDN: Using Fetch

const response = await fetch(url, {
  method: 'POST',
  mode: 'cors',
  cache: 'no-cache',
  credentials: 'same-origin',
  headers: {
    'Content-Type': 'application/json'
  },
  redirect: 'follow',
  referrerPolicy: 'no-referrer',
  body: JSON.stringify(data)
});
1 Like

jslint is now giving me this error message:

  1. Redefinition of ‘config’ from line 44.
    const config = {

How is that fixed? https://jsfiddle.net/0tsgk31z/

const videoPlayer = (function makeVideoPlayer() {
  const config = {};
  const events = {};
  const eventHandlers = {};
  let player = null;


  function loadIframeScript() {
    const tag = document.createElement("script");
    tag.src = "https://www.youtube.com/iframe_api";
    const firstScriptTag = document.getElementsByTagName("script")[0];
    firstScriptTag.parentNode.insertBefore(tag, firstScriptTag);
  }

  function onYouTubeIframeAPIReady() {
    const cover = document.querySelector(".play");
    const wrapper = cover.parentElement;
    const frameContainer = wrapper.querySelector(".video");
    videoPlayer.addPlayer(frameContainer, config.playlist);
  }

  function shufflePlaylist(player) {
    player.setShuffle(true);
    player.playVideoAt(0);
    player.stopVideo();
  }

  function onPlayerReady(event) {
    player = event.target;
    player.setVolume(100);
    shufflePlaylist(player);
    const iframe = player.h;
    iframe.dispatchEvent(events.afterPlayerReady);
  }

  function addPlayer(video, playlist) {

    const config = {
      height: 360,
      host: "https://www.youtube-nocookie.com",
      width: 640
    };
    config.playerVars = {
      autoplay: 0,
      cc_load_policy: 0,
      controls: 1,
      disablekb: 1,
      fs: 0,
      iv_load_policy: 3,
      loop: 1,
      playlist,
      rel: 0
    };
    config.events = {
      "onReady": onPlayerReady
    };

    player = new YT.Player(video, config);

    const iframe = player.h;
    const eventHandler = eventHandlers.afterPlayerReady;
    iframe.addEventListener("afterPlayerReady", eventHandler);
  }

  function play() {
    player.playVideo();
  }

  function addEvents(handler) {
    eventHandlers.afterPlayerReady = handler;
    events.afterPlayerReady = new Event("afterPlayerReady");
  }

  function init(options) {
    addEvents(options.afterPlayerReady);
    config.playlist = options.videos.join();
    loadIframeScript();
    window.onYouTubeIframeAPIReady = onYouTubeIframeAPIReady;
  }

  return {
    addPlayer,
    init,
    play
  };
}());
videoPlayer.init({
  afterPlayerReady: function initCover() {
    manageCover.init(function playVideo() {
      videoPlayer.play();
    });
  },
  videos: [
    "0dgNc5S8cLI",
    "mnfmQe8Mv1g",
    "CHahce95B1g",
    "2VwsvrPFr9w"
  ]
});

look at line 44 and figure out why you have config used twice?

I don’t see the warning.

But you do have a config object defined inside two different functions videoPlayer and addPlayer, each is local to the function scope they are declared in. If you start passing the objects around to other functions it may become unclear which of the objects it is.

BTW, I would put too much stock in JSHint it’s a bit outdated and most people would use ESLint.