Removing addRandom and arrays from the multiplayer code

We can start here.

What’s being done here is this init is supposed to give those default settings to all the other players.

managePlayer.init({
    playerVars: {
    }
  });

https://jsfiddle.net/g84k5nwb/

managePlayer.add
managePlayer.add
managePlayer.init({
managePlayer.add

It’s my understanding that the cover should be at the end, but I could be wrong.

 manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });

Now, without using console.log() I’m not able to see how the defaults are being destroyed or overwritten. So, it is hard for me to be able to understand how things should be if I am not able to see it.

console.log() is something I should be using, but adding it to the code to be able to see stuff, I need help with that.

That code has no defaults being set in the managePlayer code.

In that kind of situation, we can demonstrate one of the problems by setting a playerVars default in the managePlayer code. For example, set the start to always be at 45 seconds.

When you update the defaults object in managePlayer, you will find that this particular problem is that the managePlayer defaults in the playerVars object are not kept. The default start option is being completely removed even when no other start option is being given.

What I ask of you is to update the defaults object in the managePlayer code, so that this particular problem can be experienced.

1 Like

These are the defaults:
https://jsfiddle.net/g84k5nwb/

  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 0,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  });

I did what you said to do here:
https://jsfiddle.net/a5pqhydo/

But they are kept.

Everything works in the code as expected.

I am not experiencing any problems

  managePlayer.init({
    playerVars: {
      start:45
    }
  });

That is given to this single player.

  managePlayer.add(".playa", {
  });

This one is set to 0, and that works in the code.

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
      start:0
    }
  });

And this single video is set to 60 which works in the code.

 managePlayer.add(".playc", {
    playerVars: {
      start: 60
    }
  });
}

These are the only default playerVars / settings that are given to all the players.

Something gets changed in here, it gets sent to all the other players and I see no issues.

playerVars: {
      autoplay: 0,
      controls: 0,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3

What you did is not what I said to do.

What I said to do is in regard to the following piece of code instead.

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

I thought the whole idea of using managePlayer.init is so that the defaults aren’t used at the top in here:

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

Instead of at the top, they would be placed down at the bottom.

 managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3,
      start:45
    }
  });

Everything works as expected, I see no issues in the code.

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

const managePlayer = (function makeManagePlayer() {
  const defaults = {};
  defaults.playerVars = {
    autoplay: 0,
    controls: 1,
    disablekb: 1,
    enablejsapi: 1,
    fs: 0,
    iv_load_policy: 3,
    start:60
  };
managePlayer.add(".playa", {
    playerVars: {
      start: 45
    }
  });

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
      start: 0
    }
  });
  managePlayer.add(".playc", {
    playerVars: {
      start: 60
    }
  });
  manageCover.init({
    container: ".container",
    playButton: ".thePlay"
  });
}

In Here:

You wanted me to do this, I believe.

In here I don’t understand why the default settings would need to be in 2 spots.

Isn’t this unnecessary duplication?

Code 2
https://jsfiddle.net/aL50fg1j/1/

At the top

const managePlayer = (function makeManagePlayer() {
  const defaults = {};
  defaults.playerVars = {
    autoplay: 0,
    controls: 1,
    disablekb: 1,
    enablejsapi: 1,
    fs: 0,
    iv_load_policy: 3,
    start:45
  };

And at the bottom.

  managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3,
      start:45
    }
  });

In that code at https://jsfiddle.net/snxwhyvc/ there is no managePlayer.init statement being used at all. That does mean though that the managePlayer code is fragile, because any change that’s wanted results in being forced to edit the managePlayer code, instead of updating an init statement external to the code.

With the code at https://jsfiddle.net/aL50fg1j/1/ you have completely duplicated the managePlayer defaults with the managePlayer.init options too, which is another problem. Duplication is always best to be avoided.

I’ve updated the code so that the defaults we are not likely to want to change are in the managePlayer defaults, and we are using a managePlayer.init that helps to demonstrate the problem.

The createPlayer code is logging out the defaults, which clearly demonstrates that the defaults are being destroyed and completely replaced with the init options. That is a significant demonstration of the problem that’s happening with the defaults.

1 Like

You said you updated the code: https://jsfiddle.net/aL50fg1j/1/

It’s the same as what I posted here. Post # 20

That means the link that you posted is not updated.

You said this:

I’ve updated the code so that the defaults we are not likely to want to change are in the managePlayer defaults, and we are using a managePlayer.init that helps to demonstrate the problem.

You didn’t post the jsfiddle link to that.

Does your demonstration use console.log() in the code so everything is able to be seen?

Also,

What’s wrong with the defaults only being set by the init
https://jsfiddle.net/c75wfpye/

 managePlayer.init({
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3,
      start:45
    }
  });

And not from up here?

const managePlayer = (function makeManagePlayer() {
  const defaults = {};

Oh no, I screwed up the copy/paste and can’t get back to it now.

Never mind. Tomorrow I’ll separate out the managePlayer code and put in some tests to help demonstrate the problem. That way updates can be made to the managePlayer code so that those problems are all fixed.

1 Like

I’ve separated out the managePlayer code and have some tests in place to help ensure that the functionality of managePlayer.add and managePlayer.init keep working.

I’ve also added a test about the default issues that the managePlayer code has.

        it("shouldn't keep previous init options", function () {
            managePlayer.init({
                playerVars: {
                    start: 45
                }
            });
            managePlayer.init();
            const options = addPlayer("#cover");
            expect(options.playerVars.start).to.equal(undefined);
        });

That test helps to demonstrate a problem. That problem being when init is used a second time, the defaults from the managePlayer code should remain and the other options from previous inits shouldn’t remain.

The code is at https://jsfiddle.net/zejk8rLs/1/ and the task is to only update the managePlayer code (nothing else) so that all of the tests end up passing.

1 Like

The reason why the problem is happening is to do with the following defaults:

const managePlayer = (function makeManagePlayer() {
    const defaults = {
        playerVars: {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3
        }
    };

Those defaults are getting overwritten and replaced.whenever the init occurs.

    function init(playerOptions) {
        Object.assign(defaults, playerOptions);
    }

Those defaults cannot be allowed to get replaced.

The first parameter of Object.assign gets updated, so we start it with an empty object instead.

    function init(playerOptions) {
        Object.assign({}, defaults, playerOptions);
    }

That fixes the default issues test, but breaks one of the basic functionality tests. We need to make the Object.assign available to other parts of the code, so we can assign it to a preferred options variable.

    let preferred = {};
...
    function init(playerOptions) {
        preferred = Object.assign({}, defaults, playerOptions);
    }

and we need the other parts of the code to refer to that preferred variable instead of defaults.

    function createPlayer(videoWrapper, settings = {}) {
        const video = videoWrapper.querySelector(".video");
        const playerOptions = combinePlayerOptions(preferred, settings);

That nearly works. It’s expected that the defaults are available even when init hasn’t been run, so we can have preferred start with a copy of the defaults.

    let preferred = {...defaults};

That causes all of the tests to pass https://jsfiddle.net/rtvxs95y/ but our work on the code is not finished yet. Getting the tests to pass is literally the least that can be done. Once they are passing we should continue to work on the code, refactoring it without breaking the tests, to clean up problems in the code, which I’ll do next.

1 Like

When it comes to refactoring the code, I want to put both the defaults and the preferred into one object called options.

const managePlayer = (function makeManagePlayer() {
        const defaultOptions = {
            playerVars: {
                autoplay: 0,
                controls: 1,
                disablekb: 1,
                enablejsapi: 1,
                fs: 0,
                iv_load_policy: 3
            }
        };
        const options = {
            defaults: defaultOptions,
            preferred: defaultOptions
        };

That means updating the two things that access defaults and preferred:

    function createPlayer(videoWrapper, settings = {}) {
        ...
        const preferred = options.preferred;
        const combined = combinePlayerOptions(preferred, settings);
...
    function init(playerOptions) {
        const defaults = options.defaults;
        options.preferred = Object.assign({}, defaults, playerOptions);
    }

While doing that I notice that some function parameters are called settings, which isn’t really correct. They are playerOptions instead so let’s update those.

    function createPlayer(videoWrapper, playerOptions = {}) {
        ...
        const combined = combinePlayerOptions(preferred, playerOptions);
...
    function createCoverClickHandler(playerOptions) {
        ...
            const player = createPlayer(wrapper, playerOptions);
        };
    }
...
    function addPlayer(coverSelector, playerOptions) {
        const clickHandler = createCoverClickHandler(playerOptions);

And now the managePlayer code is much better than it ever was before.
https://jsfiddle.net/rtvxs95y/1/

1 Like

Are the tests/refactoring done and the code ready to be updated with the new code?

It seems okay for now. The good news is that if any other behaviour is expected from the managePlayer code, we can add another test for that expected behaviour, and update managePlayer to achieve that new thing while also keeping all of the other tests working too.

1 Like

Can you provide a jsfiddle link to the updated new code? One that is a working code without the tests in it. And thank you for your help with helping me understand this.

It is completely worthless to have only the function by itself on a JSFiddle page with no other supporting tests to ensure that everything works properly. The managePlayer code can be easily copied from the final code that I have at the above link, and if you really are desperately keen for a separate page it’s the code all by itself then that is easily done by yourself too.

Is the test code:
https://jsfiddle.net/rtvxs95y/1/

Not ready to be added to the full code?
https://jsfiddle.net/aL50fg1j/1/

If more tests are needed, then more tests can be done and I will wait.

To put the test code inside the full code, all the test code stuff would need to be removed from the test code to be able to be placed in the full code, and I don’t know how to remove the test code stuff.

Is this all I am doing?

This was my attempt at adding the test code to the full code.

Did I do this right?

If no, then this is something I need help with.
https://jsfiddle.net/vmgk5nce/1/

const managePlayer = (function makeManagePlayer() {
  const defaultOptions = {
    playerVars: {
      autoplay: 0,
      controls: 1,
      disablekb: 1,
      enablejsapi: 1,
      fs: 0,
      iv_load_policy: 3
    }
  };
  const options = {
    defaults: defaultOptions,
    preferred: defaultOptions
  };

  function show(el) {
    el.classList.remove("hide");
  }

  function createPlayer(videoWrapper, playerOptions = {}) {
    const video = videoWrapper.querySelector(".video");
    const preferred = options.preferred;
    const combined = combinePlayerOptions(preferred, playerOptions);
    return videoPlayer.addPlayer(video, combined);
  }

  function createCoverClickHandler(playerOptions) {
    return function coverClickHandler(evt) {
      const cover = evt.currentTarget;
      const wrapper = cover.nextElementSibling;
      show(wrapper);
      const player = createPlayer(wrapper, playerOptions);
      wrapper.player = player;
    };
  }

  function addPlayer(coverSelector, playerOptions) {
    const clickHandler = createCoverClickHandler(playerOptions);
    manageCover.addCoverHandler(coverSelector, clickHandler);
  }

  function init(playerOptions) {
    const defaults = options.defaults;
    options.preferred = Object.assign({}, defaults, playerOptions);
  }
  return {
    add: addPlayer,
    init
  };
}());

function onYouTubeIframeAPIReady() {

  managePlayer.add(".playa", {});

  managePlayer.add(".playb", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g",
      start: 50
    }
  });

  managePlayer.add(".playc", {
    playerVars: {
      start: 60
    }
  });
}

manageCover.init({
  container: ".container",
  playButton: ".thePlay"
});

It is only the managePlayer code that you should take from the test page. Everything else there is not for you and is only there to support the testing.

1 Like

After adding the Top in

const managePlayer = (function makeManagePlayer() {
    const defaultOptions = {
        playerVars: {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3
        }
    };
    const options = {
        defaults: defaultOptions,
        preferred: defaultOptions
    };

    function show(el) {
        el.classList.remove("hide");
    }

    function createPlayer(videoWrapper, playerOptions = {}) {
        const video = videoWrapper.querySelector(".video");
        const preferred = options.preferred;
        const combined = combinePlayerOptions(preferred, playerOptions);
        return videoPlayer.addPlayer(video, combined);
    }

    function createCoverClickHandler(playerOptions) {
        return function coverClickHandler(evt) {
            const cover = evt.currentTarget;
            const wrapper = cover.nextElementSibling;
            show(wrapper);
            const player = createPlayer(wrapper, playerOptions);
            wrapper.player = player;
        };
    }

    function addPlayer(coverSelector, playerOptions) {
        const clickHandler = createCoverClickHandler(playerOptions);
        manageCover.addCoverHandler(coverSelector, clickHandler);
    }

    function init(playerOptions) {
        const defaults = options.defaults;
        options.preferred = Object.assign({}, defaults, playerOptions);
    }
    return {
        add: addPlayer,
        init
    };
}());

Bottom

This is the section where I get confused the most about.

I don’t know if I did this right.

https://jsfiddle.net/7a9bmpdf/

function onYouTubeIframeAPIReady() {

    managePlayer.add(".playa", {});

    managePlayer.init({
        playerVars: {
            autoplay: 0,
            controls: 1,
            disablekb: 1,
            enablejsapi: 1,
            fs: 0,
            iv_load_policy: 3
        }
    });

    managePlayer.add(".playb", {
        playerVars: {
            playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
        }
    });

    managePlayer.add(".playc", {
        playerVars: {
            start: 46
        }
    });

    manageCover.init({
        container: ".container",
        playButton: ".thePlay"
    });
}