Questions about the YouTube player_api code


#253

No, that won’t serve any benefit at all.


#254

Thank you…


#255

If I deleted the last line of the code.

This Line:
return combinedSettings;
https://jsfiddle.net/zt7anuL3/96/

And I replaced it with any of these other lines.

Would using any of the other lines be better?

If I replaced it with this line:
return Object.assign(newSettings, oldSettings);
https://jsfiddle.net/zt7anuL3/103/

or this line:
return Object.assign(combinedSettings);
https://jsfiddle.net/zt7anuL3/106/

or this line:
return Object.assign(oldSettings);
https://jsfiddle.net/zt7anuL3/110/

or this line
return oldSettings;
https://jsfiddle.net/zt7anuL3/108/

or this line
return newSettings, oldSettings;
https://jsfiddle.net/zt7anuL3/112/


or should I leave this line there?
return combinedSettings;
https://jsfiddle.net/zt7anuL3/96/

Because it works with all the other ending lines too.

Is there one ending line that would be best to use there?

or keep it the way it is, using:
return combinedSettings;

    function combineSettings(oldSettings, newSettings) {
        const props = Object.keys(newSettings);
        const combinedSettings = props.reduce(function combine(combined, prop) {
            if (typeof(oldSettings[prop]) === "object") {
                const oldProp = oldSettings[prop] || {};
                const newProp = newSettings[prop] || {};
                combined[prop] = combineSettings(oldProp, newProp);
            } else {
                combined[prop] = newSettings[prop];
            }
            return combined;
        }, oldSettings);

        return combinedSettings;

      // or return Object.assign(newSettings, oldSettings);
      // or return Object.assign(combinedSettings);
      // or return Object.assign(oldSettings);
      // or return newSettings, oldSettings;
      // or return oldSettings;
    }

#256

There are problems that each of those other ones have, that I do not currently have time to delve in to.


#257

That’s okay, I just wanted to know if any of these other ending lines would’ve been better to use, and you say there are problems with each of them.

      return Object.assign(newSettings, oldSettings);
      return Object.assign(combinedSettings);
      return Object.assign(oldSettings);
      return newSettings, oldSettings;
      return oldSettings;

So, then it is best to use what was originally with the code, which was this:
return combinedSettings;

And that I shouldn’t change it to something else.


#260

Do you like how this one is set up, or no?

And if no, why is it bad?
And would it be able to be improved at all?

Is it doing what we want it to be doing, yes.
It combines the oldSettings, with the newSettings.
Can it be written better than how it is written here, I don’t know.
It might be able to be improved.
https://jsfiddle.net/Lnj7quby/6/

    function combineSettings(oldSettings, newSettings) {
        const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);
        const settings = Object.assign({}, oldSettings, newSettings);
        settings.playerVars = playerVars;
        return settings;
    }

#261

The only issue I see with the code is this line:
const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);

Because it’s longer than 80 characters, and jslint doesn’t like it.

Would there be a way to make that line shorter, which I don’t know how that would be possible to do, that or improve the overall code?

Because the way this is set up seems simple to use.

It’s just that line which is longer than 80 characters that I don’t know how to fix.
But in order to fix that line, the whole thing might need to be re-done, but I’m not sure.
https://jsfiddle.net/89zwf62y/2/

function combineSettings(oldSettings, newSettings) {
    const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);
    const settings = Object.assign({}, oldSettings, newSettings);
    settings.playerVars = playerVars;
    return settings;
}

It could be written like this, but then it’s not readable:
Changing Settings, to just the letter S
https://jsfiddle.net/89zwf62y/3/

function combineSettings(oldS, newS) {
    const playerVars = Object.assign({}, oldS.playerVars, newS.playerVars);
    const settings = Object.assign({}, oldS, newS);
    settings.playerVars = playerVars;
    return settings;
}

#262

That works when playerVars is the only object that has new settings. When other objects are updated though such as with new events, then that code fails to work.


#263

Yes, just split the line.

    const playerVars = Object.assign({},
        oldSettings.playerVars, newSettings.playerVars);

#264

Or, assign them to variables.

const oldPlayerVars = oldSettings.playerVars;
const newPlayerVars = newSettings.playerVars;
const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);

#265

Where would these go then?

settings.playerVars = playerVars;
return settings;
function combineSettings(oldSettings, newSettings) {
const oldPlayerVars = oldSettings.playerVars;
const newPlayerVars = newSettings.playerVars;
const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
 }

#266

At the end.


#267

It’s not working in the code:
https://jsfiddle.net/89zwf62y/6/


function combineSettings(oldSettings, newSettings) {
const oldPlayerVars = oldSettings.playerVars;
const newPlayerVars = newSettings.playerVars;
const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
settings.playerVars = playerVars;
return settings;
 }

#268

Where is the settings object defined?


#270

const settings = Object.assign({}, oldSettings, newSettings);

Is missing from here:

function combineSettings(oldSettings, newSettings) {
const oldPlayerVars = oldSettings.playerVars;
const newPlayerVars = newSettings.playerVars;
const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
settings.playerVars = playerVars;
return settings;
 }

#271

After breaking down this line:
const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);

Into this:

    const oldPlayerVars = oldSettings.playerVars;
    const newPlayerVars = newSettings.playerVars;
    const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);

These lines needed to be added on:

    const settings = Object.assign({}, oldSettings, newSettings);
    settings.playerVars = playerVars;
    return settings;

To form this:
https://jsfiddle.net/89zwf62y/13/

function combineSettings(oldSettings, newSettings) {
    const oldPlayerVars = oldSettings.playerVars;
    const newPlayerVars = newSettings.playerVars;
    const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
    const settings = Object.assign({}, oldSettings, newSettings);
    settings.playerVars = playerVars;
    return settings;
}

#272

There’s an inaccurate assumption happening there. There is no cause and effect with breaking down the line.

The first line of code that you were concerned with didn’t work regardless of whether it was broken down or not. Those added-on lines are required both before the line was broken down, and after.


#273

With this line removed it doesn’t work:
const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);
https://jsfiddle.net/89zwf62y/16/

 function combineSettings(oldSettings, newSettings) {
        const settings = Object.assign({}, oldSettings, newSettings);
        settings.playerVars = playerVars;
        return settings;
    }

With that line added, it works:
https://jsfiddle.net/89zwf62y/2/

    function combineSettings(oldSettings, newSettings) {
        const playerVars = Object.assign({}, oldSettings.playerVars, newSettings.playerVars);
        const settings = Object.assign({}, oldSettings, newSettings);
        settings.playerVars = playerVars;
        return settings;
    }

What do you mean?


#274

I’ll just know that this is how it would be written the right way:

And thank you for helping me fix this one.
https://jsfiddle.net/89zwf62y/13/

function combineSettings(oldSettings, newSettings) {
    const oldPlayerVars = oldSettings.playerVars;
    const newPlayerVars = newSettings.playerVars;
    const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
    const settings = Object.assign({}, oldSettings, newSettings);
    settings.playerVars = playerVars;
    return settings;
}

#275

Further improvements. Move settings up above playerVars.

function combineSettings(oldSettings, newSettings) {
    const oldPlayerVars = oldSettings.playerVars;
    const newPlayerVars = newSettings.playerVars;
    const settings = Object.assign({}, oldSettings, newSettings);
    const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
    // const settings = Object.assign({}, oldSettings, newSettings);
    settings.playerVars = playerVars;
    return settings;
}

That then lets you assign playerVars only once.

function combineSettings(oldSettings, newSettings) {
    const oldPlayerVars = oldSettings.playerVars;
    const newPlayerVars = newSettings.playerVars;
    const settings = Object.assign({}, oldSettings, newSettings);
    // const playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
    // settings.playerVars = playerVars;
    settings.playerVars = Object.assign({}, oldPlayerVars, newPlayerVars);
    return settings;
}