Adding Const defaultPlayerVars =

I think that it’s going to help to clearly define the terms.

  • playerVars is the most clear, that defines several types of behaviour for the player.
  • Youtube says about the second parameter given to add a player: “The second parameter is an object that specifies player options.” so calling that object options or playerOptions makes good sense.
  • playerVars is a parameter of that options object.
  • The settings object combines both playerVars and options parameters for the sake of convenience. That combination really needs to be separated before using them.

When those terms are being used for different types of things, that is where confusion easily occurs. That confusion should be removed at the earliest convenience.

With that out of the way, how I would add in that other initPlayer function is by renaming the old one to oldInitPlayer and having the new one in there too as initPlayer.

That way getting things to work with the new initPlayer function can then occur in relative safety, and when things work with it the oldInitPlayer can then be removed.

If no settings are given to initPlayer then we don’t want that to change from the default settings. The function should still be able to run even with no settings though, so we can tell the function to use an empty object by default when no settings are given.

function initPlayer(videoWrapper, settings = {}) {

Because we want those settings to only be for the player options and playerVars, we need to remove other things that don’t belong there after using them.

    const cover = document.querySelector(opts.target);
    delete opts.target;
    cover.addEventListener("click", coverClickHandler);

Currently the loadPlayer function calls its parameter opts. Those are not just options for the video player though. They are also configuration information for the cover. That means that we should call the loadPlayer parameter something other than opts, for which cover is a good choice.

That way information flows like this:

config => cover & settings
settings => options & playerVars

When renaming opts to config, I’ll keep the oldInitPlayer working by passing in the config, that it receives as opts.

function loadPlayer(config) {
    ...
    function oldInitPlayer(wrapper, opts) {
        ...
    }

    function coverClickHandler(evt) {
        ...
        show(wrapper);
        oldInitPlayer(wrapper, config);
    }

    const cover = document.querySelector(config.target);
    delete config.target;
    cover.addEventListener("click", coverClickHandler);
}

Now is a good time to switch things over from oldInitPlayer to the new initPlayer.

        show(wrapper);
        initPlayer(wrapper, config);

In the initPlayer function is now a good time to rename variables to options so that they fall in line with the above naming guidelines.

        function paramInOptions(options, param) {
            if (settings[param] !== undefined) {
                options[param] = settings[param];
                delete settings[param];
            }
            return options;
        }
...
        const optionParams = ["width", "height", "videoid", "host"];
        const playerOptions = optionParams.reduce(paramInOptions, {});
...
        playerOptions.playerVars = playerVars;
        const player = videoPlayer.addPlayer(video, playerOptions);

I console.log some variables to help ensure that they are behaving themself. The playerVars object is misbehaving.

{
  autoplay: 0
  controls: 1
  disablekb: 1
  enablejsapi: 1
  fs: 0
  height: 207
  iv_load_policy: 3
  rel: 0
  width: 277
}

The width and height are inappropriately defined in the playerVar defaults. Those should be separately defined as playerOption defaults.

        const defaultPlayerOptions = {
            height: 207,
            width: 277
        };

That way, once we’ve got the preferred options for the player, we can combine them together with the default options. If the preferred ones aren’t specified in the config then the default ones will take place instead.

        const optionParams = ["width", "height", "videoid", "host"];
        const preferredOptions = optionParams.reduce(paramInOptions, {});
        const playerOptions = Object.assign({}, defaultPlayerOptions, preferredOptions);

The width and height are still inappropriately in playerVars. We need to create that after the player options, so that the remainder can then be assigned to playerVars. That should be as easy as moving the playerVars code down below the options code.

        const playerOptions = optionParams.reduce(paramInOptions, {});
        const playerVars = Object.assign({}, defaultPlayerVars, settings);

It becomes clear now that the job of initPlayer is to crate a properly structured set of player options, and use that to add a player. That information is useful enough and not easily represented by the code, that we should add it as a comment to the function.

    function initPlayer(videoWrapper, settings = {}) {
        // Create a properly structured player options object
        // and use it to add a player.

We can now get rid of oldInitPlayer, and the updated code is at https://jsfiddle.net/bg731cwo/

1 Like