Removing unnecessary duplication

JavaScript
1

I’m trying to remove these playerVars from the default settings because it’s unnecessary duplication.

I’m not able to figure out how this would be done.

    start: 0,
    end: 999999,
    loop: true,

https://jsfiddle.net/hzyrfkwb/461/

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,

Would there be a way where I would only add them if they are needed?

As was done in this example:
https://jsfiddle.net/hzyrfkwb/432/

Doing this would remove a lot of unnecessary duplication.

loadPlayer({
   target: ".jacketc",
   width: 600,
   height: 338,
   playerVars: {
      start: 200,
      end: 205,
      loop: true
   }
});

It doesn’t need to be stated in both the top:

  function addVideo(video, settings) {
    const defaultSettings = {
      width: settings.width || 640,
      height: settings.height || 390,
      videoId: video.dataset.id,
      playerVars: {
        start: 0,
        end: 999999,
        loop: true,

And also at the bottom:

loadPlayer({
   target: ".jacketc",
   width: 600,
   height: 338,
   playerVars: {
      start: 200,
      end: 205,
      loop: true
   }
});

These should only be added to loadPlayer if they are needed.
They shouldn’t be required to be at the top in the default settings.

        start: 0,
        end: 999999,
        loop: true,

These should only be the default settings:

         autoplay: 1,
         controls: 1,
         showinfo: 1,
         rel: 0,
         iv_load_policy: 3,
         cc_load_policy: 0,
         fs: 0,
         disablekb: 1
      };

These would only be added to loadPlayer if they are needed.
If they are not needed they don’t need to appear in the javascript.

        start: 0,
        end: 999999,
        loop: true,

How would I be able to implement this adjustment to the code?
https://jsfiddle.net/hzyrfkwb/453/

Where these playerVars aren’t needed in the default settings.
They only get added to loadPlayer if they are needed.

        start: 200,
        end: 205,
        loop: true,

That’s exactly how it works in the other code.
https://jsfiddle.net/hzyrfkwb/465/

How would I be able to implement that in the new, updated code?
https://jsfiddle.net/hzyrfkwb/453/

2

The problem is in the way you wrote combineSettings. The logic you have goes:

  • If the given property is an object, recursively call combineSettings().
  • If the given property of defaultPlayerVars also exists on playerVars, then merge the two, using the custom value.

That’s it. So you don’t actually get properties of playerVars that don’t exist on defaultPlayerVars. Perhaps, you may want to get the Object.keys of playerVars, and update or add those properties on defaultPlayerVars.

So the logic might be:

  • Clone the defaultPlayerVars as is.
  • Get all the Object keys of playerVars.
  • Set each object property FROM playerVars on the defaultPlayerVars clone. This would add the properties if they don’t exist, or replace them if they do.
3

I’m a little confused on how to do this, all help is appreciated.

Step 1

Clone the defaultPlayerVars as is.
That would be this, right:
https://jsfiddle.net/hzyrfkwb/532/

    function addVideo(video, settings) {
        const defaultSettings = {
            width: settings.width || 640,
            height: settings.height || 390,
            videoId: video.dataset.id,
      const defaultPlayerVars = {
         autoplay: 1,
         controls: 1,
         showinfo: 1,
         rel: 0,
         iv_load_policy: 3,
         cc_load_policy: 0,
         fs: 0,
         disablekb: 1
      };
            events: {
                "onReady": onPlayerReady,
                "onStateChange": onPlayerStateChange
            }
        };
4

and passing that into your combineSettings function, yes. The big issue, I think, is that your combineSettings is only taking properties from the defaultPlayerVars – it doesn’t look to see if properties exist on playerVar that DON’T on exist on the defaults.

5

You can call loadPlayer on players with the same inputs in a loop to reduce some duplication. That said maybe you want player vars to be able to be changed later.

for (let i = "e".charCodeAt(0); i <= "i".charCodeAt(0); i++) {
	loadPlayer({
		target: `.play${String.fromCharCode(i}}`,
		playerVars: {
			start: 1,
			end: 50
		}
	});
}

I wouldn’t rush to reduce duplication unless you are sure things will stay the same form.

6

Would you be able to put together a working jsfiddle of your recommendations please.
I’ve been trying at this for some time now.
https://jsfiddle.net/hzyrfkwb/532/

7

Done and done. I also took the time to do the object deconstruction thing I’d mentioned. Here’s how that code works:

(within loadPlayer() )

    function initPlayer(wrapper) {
        const video = wrapper.querySelector(".video");
        // Create an EMPTY object, this will be our settings.
        let settings = {};
        // Strip out the width and height from the passed object, leave the rest
        const {width, height, ...options} = opts
        // default to 198x198 for multiple video players
        settings.width = width || 198;
        settings.height = height || 198;
        
        // the var 'options' contains everything BUT width and height.
        settings.playerVars = options.playerVars || options;
        
        videoPlayer.init(video, settings);
    }

Doing the above means that width and height are not being added to both the settings object AND to playerVars. Now, within the combineSettings function, I changed it to this:

    function combineSettings(oldSettings, newSettings) {
        // We want to add or update only those properties found on our 
        //  newSettings object -- this is our custom settings. So get ONLY
        //  those keys.
        const props = Object.keys(newSettings);
        let combinedSettings =  props.reduce(function combine(combined, prop) {
            // Is the current property a  nested object? If so, RECURSE!
            if (typeof(oldSettings[prop]) === "object") {
                const oldProp = oldSettings[prop] || {};
                const newProp = newSettings[prop] || {};
                combined[prop] = combineSettings(oldProp, newProp);
            } else {
                // Otherwise, simply add/update the property on  our defaults
                combined[prop] = newSettings[prop];
            }
            return combined;
        }, oldSettings);

        return combinedSettings;
    }

Big change is that, as I am using the original oldSettings object, I really don’t care about its properties. I simply want to add/update those properties coming in from newSettings.

Thus, we have this fiddle: https://jsfiddle.net/snowMonkey/fmjh9ce3/

1 Like
8

If you open the console, you will see the final settings object. Note that width and height are in the root level, while start and end are only in the playerVars IF THEY HAVE BEEN DEFINED.

Good luck, going afk.

1 Like
9

Thank you, much appreciated.

10

Is there a reason why you put 3 dots here, is that part of the code, or can they be removed?
const {width, height, ...options} = opts

This is coming up in jslint:
https://jsfiddle.net/zt7anuL3/9/

        const {
            width,
            height,
            ...options
        } = opts;

How would this be fixed?

11

Google ‘javascript spread operator’. You may need to research jslint and ES6.

1 Like
12

Someone told me this:

This is very wrong, you are creating a new const options that will probably be undefined as there is probably not an options property on opts, …options creates a new const that is the remaining properties left over after width and height have been assigned to consts.

In reference to this line:

        const {
            width,
            height,
            ...options
        } = opts;

Does this person not know what they are talking about?

13

Their first sentence says one thing and their second the opposite. I don’t know them, so don’t know what they do or don’t know.

That said, in the context i used it, …options gathers all remaining properties and assigns them to options. They say it doesn’t do that, but check the console logs. Its clearly working.

1 Like