Remove manageCover from the managePlayer code

We can work on this next?

remove manageCover from the managePlayer function

https://jsfiddle.net/koLvg1p4/

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

In Post #94 you said after your previous instructions were completed, and in Post #96 I think you just confirmed that I completed them.

When updating the UML diagram I realized that I had missed a connection from managePlayer to manageCover. To correct things, I’ve made a list of what connects to each module.

I find that it’s most reliable to first do a wordsearch for each module in order of how they appear in the code, manageUI first, then manageCover, videoPlayer, and managePlayer.

  • onYouTubeIframeAPIReady → manageUI
  • managePlayer → manageCover
  • onYouTubeIframeAPIReady → manageCover
  • managePlayer → videoPlayer
  • onYouTubeIframeAPIReady → managePlayer

Ordered by how they appear in the code they are:

  • managePlayer → videoPlayer
  • managePlayer → manageCover
  • onYouTubeIframeAPIReady → managePlayer
  • onYouTubeIframeAPIReady → manageCover
  • onYouTubeIframeAPIReady → manageUI

And the order that they are executed in the code is:

  • onYouTubeIframeAPIReady → managePlayer
  • managePlayer → manageCover
  • onYouTubeIframeAPIReady → manageCover
  • onYouTubeIframeAPIReady → manageUI
  • managePlayer → videoPlayer

Here is an updated UML diagram showing that inappropriate connection from managePlayer to manageCover.

The managePlayer code should only have knowledge about the player. It’s a breach of the Separation of Concerns policy that we have in programming, for the managePlayer code to know about other modules.

It is that connection from managePlayer to manageCover that we are going to work on removing, which I’ll delve into in my next post.

1 Like

In regard to managePlayer inappropriately calling manageCover, here is the code in question:

From the managePlayer module in https://jsfiddle.net/koLvg1p4/

    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);
    }

A few problems are noticed there.

  • The createCoverClickHandler function is in managePlayer when it should be in manageCover
  • The createCoverClickHandler function does things both for the cover and the player. Those need to be separated.
  • The managePlayer code has intimate knowledge of what the cover needs to do. That knowledge needs to be in the manageCover code instead.

In terms of big picture, we are going to move those functions out of managePlayer and in to the onYouTubeIframeAPIReady code. That way we can then reorganise the code and put things away where they belong in terms of the cover and the player.

Before moving out the code though, we need to ensure that functions that are called from the code, are available no matter where we move the code to. That means temporarily making public functions such as show and createPlayer.

Step 1: Change needed private functions to be public
Step 2: Move unsuitable functions to onYouTubeIframeAPIReady
Step 3: Rename managePlayer.add to use the addPlayer function in onYouTubeIframeAPIReady
Step 4: Move parts of the functions back in to manageCover and managePlayer
Step 5: Rename addPlayer in onYouTubeIframeAPIReady back to managePlayer.add
Step 6: Change public functions that no longer need to be public back to private
Step 7: Remove as much of the onYouTubeIframeAPIReady functions as we can

And after all of that, we re-evaluate what we’ve done to figure out if any good improvements can be made from there.

I will explain what we do for Step 1 in my next post.

1 Like

Thank you for putting together the steps needed to do this.

I will wait for your next post.

Step 1: Change needed private functions to be public

In the above code, I’ll use the terms private methods and public methods to talk about functions that can’t and can be accessed from outside of the module.

The private methods used by the above functions are show(), createPlayer(), and createCoverClickHandler(). We need to make those private methods public, so that the functions will still work when we move them out.

It is at the end of the managePlayer code where the public methods are managed:

    return {
        add: addPlayer
    };

Currently the add method is the only public method (which uses the private method called addPlayer). We need to add show, createPlayer, and createCoverClickHandler to that list of public methods.

Of the key value pair, where add is the key and addPlayer is the value, looking like add: addPlayer, when both the key and the value are both the same, only the value is needed to be used. That is a property shorthand that helps to avoid annoying createPlayer: createPlayer duplications in the objet properties.

Some people prefer to move out the functions first and use the errors to help inform them about what needs to be done to fix things from there. You though have expressed a desire to avoid errors while doing this work, which means extra overhead and thinking ahead to avoid any potential errors that may occur.

After the public methods have been added for show, createPlayer, and createCoverClickHandler, we need to update those functions so that the public methods are used instead. That should only mean updating function calls in those functions such as show, so that managePlayer.show is used instead.

That is the preparation that needs to be done for Step 1, before Step 2 can be achieved without error.

1 Like

This? https://jsfiddle.net/cxfa3tqw/

  return {
    createPlayer,
    createCoverClickHandler,
    show: addPlayer
  };

}());

function onYouTubeIframeAPIReady() {

  managePlayer.show(".playa", {});
  managePlayer.show(".playb", {});
  managePlayer.show(".playc", {});
  managePlayer.show(".playd", {});
  managePlayer.show(".playe", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  managePlayer.show(".playe", {});
  managePlayer.show(".playf", {});
  managePlayer.show(".playg", {});
  managePlayer.show(".playh", {});
  managePlayer.show(".playi", {});

No, that is quite the wrong approach. None of the onYouTubeIframeAPIReady code needs to change yet, and show needs to be a separate public method similar to createPlayer. There is absolutely no need to change the public method called add either. That should remain as it was, referring to the addPlayer function.

1 Like

You said this

so that managePlayer.show is used instead.

So, I thought I was supposed to do this:
managePlayer.show(".playa", {});

Then, is this what you wanted? https://jsfiddle.net/8bsmq69o/

jslint wanted me to put the returns in abc order so I obliged.

return {
        add: addPlayer,
        createCoverClickHandler,
        createPlayer,
        show
    };
}());

function onYouTubeIframeAPIReady() {

    managePlayer.add(".playa", {});
    managePlayer.add(".playb", {});
    managePlayer.add(".playc", {});
    managePlayer.add(".playd", {});
    managePlayer.add(".playe", {
        playerVars: {
            playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
        }
    });
    managePlayer.add(".playe", {});
    managePlayer.add(".playf", {});
    managePlayer.add(".playg", {});
    managePlayer.add(".playh", {});
    managePlayer.add(".playi", {});

I don’t unserstand what I am supposed to do in Step 2

Step 2: Move unsuitable functions to onYouTubeIframeAPIReady

Which functions are unsuitable?

Please read what I have written. The information is there for your benefit. I have written extensively about the two undesirable functions in the post detailing the steps, and in my subsequent post after that.

Please reread them if you have to. Can you figure out which two functions are the undesirable ones from those two posts?

1 Like
  • The createCoverClickHandler function is in managePlayer when it should be in manageCover
  • The createCoverClickHandler function does things both for the cover and the player. Those need to be separated.

Shouldn’t I be doing this instead then, or no?

Adding it to manageCover?

Maybe I am not up to that yet.

const manageCover = (function makeManageCover() {

    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;
        };
    }

Maybe that is Step 4.

All instructions for Step 2 - Step 7 can be found in Post #3 and Post #5?

I think there are only instructions on how to do Step 1, which can be found in Post #5.

I see: Step 1

Step 1: Change needed private functions to be public

I don’t see Step 2

Am I supposed to be waiting for those before going ahead?

Should I be waiting for further instruction, guidance?

Sometimes I think I can go ahead on my own, thinking I know what to do, then I end up getting it all messed up and wrong because I am doing too many things at once.

I asked because I am stuck here.

There is an issue I run into when moving 2 functions into onYouTubeIframeAPIReady.

The code stops working: https://jsfiddle.net/7toykpu2/1/

I am stuck and confused.

These are the 2 functions being moved.

Is that wrong?

  function createCoverClickHandler(playerOptions) {

  function addPlayer(coverSelector, playerOptions) {

After moving those 2 functions out, should the code still be working right away?

or, am I supposed to be making other changes also?

The return I think should only have show in it I think.

This

  return {
  show
  };

Not this:

  return {
    add: addPlayer,
    createCoverClickHandler,
    createPlayer,
    show
  };

I tried:
add: show Did not work.
show: show Did not work.
addPlayer: show Did not work.

What am I doing wrong?

I don’t know how to move the functions out without the code falling apart.

I can’t do anything without making other changes, and I don’t know what to do next.

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

  function combinePlayerOptions(opts1 = {}, opts2 = {}) {
    const combined = Object.assign({}, opts1, opts2);
    Object.keys(opts1).forEach(function checkObjects(prop) {
      if (typeof opts1[prop] === "object") {
        combined[prop] = Object.assign({}, opts1[prop], opts2[prop]);
      }
    });
    return combined;
  }

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

  return {
    add: addPlayer,
    createCoverClickHandler,
    createPlayer,
    show
  };
}());

function onYouTubeIframeAPIReady() {

  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);
  }
  
  managePlayer.show(".playa", {});
  managePlayer.show(".playb", {});
  managePlayer.show(".playc", {});
  managePlayer.show(".playd", {});
  managePlayer.show(".playe", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  
  managePlayer.show(".playe", {});
  managePlayer.show(".playf", {});
  managePlayer.show(".playg", {});
  managePlayer.show(".playh", {});

I am stuck here:
Code does not work: https://jsfiddle.net/shkb3eLj/

There are no instructions that tell me what to do after moving the functions out.

Am I supposed to wait for further instructions?

I don’t see Step 2 Instructions.

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

  return {
    createPlayer,
    show
  };
}());

function onYouTubeIframeAPIReady() {

  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);
  }
  
  managePlayer.show(".playa", {});
  managePlayer.show(".playb", {});
  managePlayer.show(".playc", {});
  managePlayer.show(".playd", {});
  managePlayer.show(".playe", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  
  managePlayer.show(".playe", {});
  managePlayer.show(".playf", {});
  managePlayer.show(".playg", {});
  managePlayer.show(".playh", {});

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

  manageUI.init({});
}

Are these still staying as add, not show?
I’m confused.

I don’t know how to get the code to work.

Changed back to add:
https://jsfiddle.net/kxhg3pr8/

  managePlayer.add(".playa", {});
  managePlayer.add(".playb", {});
  managePlayer.add(".playc", {});
  managePlayer.add(".playd", {});
  managePlayer.add(".playe", {
    playerVars: {
      playlist: "0dgNc5S8cLI,mnfmQe8Mv1g,-Xgi_way56U,CHahce95B1g"
    }
  });
  
  managePlayer.add(".playe", {});
  managePlayer.add(".playf", {});
  managePlayer.add(".playg", {});
  managePlayer.add(".playh", {});

Please only do step 1, and have me check that you have done things correctly before we move on to step 2.

1 Like

I completed Step 1 here: https://jsfiddle.net/9qz7bujL/

    return {
        add: addPlayer,
        createCoverClickHandler,
        createPlayer,
        show
    };
}());

If step 1 has been completed, then step 2 and 3 can next be done.
I don’t think that you’ve completed step 1 though because trouble happens when you try steps 2 and 3.

I’m confused and lost.

I keep reading and re-reading your instructions and nothing is telling me to write the returns differently from what I put down in my attempts here.

I still haven’t gotten past Step 1 then.

You want me to do this?

If no, I’m having trouble with the code and need help.

This is where I am having difficulty with the code.

https://jsfiddle.net/gcfkwmLr/1/

 return {
        add: addPlayer,
        createCover: createCoverClickHandler,
        create: createPlayer,
        show
    };
}());

I don’t seem to understand what the structure of the returns should be.

I was told this is wrong: https://jsfiddle.net/9qz7bujL/
Post #16

    return {
        add: addPlayer,
        createCoverClickHandler,
        createPlayer,
        show
    };
}());

I was told this is wrong: https://jsfiddle.net/gtc0e7hf/
Post #8

 return {
    show: addPlayer,
    createCoverClickHandler,
    createPlayer,
  };

I am thinking this is wrong also. https://jsfiddle.net/6yz7xo18/1/

  return {
    add: addPlayer,
    createCover: createCoverClickHandler(),
    create: createPlayer(),
    show
  };

What am I supposed to do in the code that is supposed to prepare me to move out these functions?

 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);
  }

I still am having a lot of difficulty with Step 1.

If I had the returns structured correctly, I would be able to move out the functions.

All the returns I have written turn out to be wrong.

Where am I making my mistakes?

What am I doing that I shouldn’t be doing, and what should I be doing instead?

I keep reading and re-reading this over and over again.

I don’t understand what I am doing wrong.

Maybe it is not possible to move out those functions.

Step 1: Change needed private functions to be public

In the above code, I’ll use the terms private methods and public methods to talk about functions that can’t and can be accessed from outside of the module.

The private methods used by the above functions are show(), createPlayer(), and createCoverClickHandler(). We need to make those private methods public, so that the functions will still work when we move them out.

It is at the end of the managePlayer code where the public methods are managed:

    return {
        add: addPlayer
    };

Currently the add method is the only public method (which uses the private method called addPlayer). We need to add show, createPlayer, and createCoverClickHandler to that list of public methods.

Of the key value pair, where add is the key and addPlayer is the value, looking like add: addPlayer , when both the key and the value are both the same, only the value is needed to be used. That is a property shorthand that helps to avoid annoying createPlayer: createPlayer duplications in the objet properties.

All of these are wrong.

I don’t know, or can’t think of any possible other way the return statements can be written differently.

    return {
        add: addPlayer,
        createCoverClickHandler,
        createPlayer,
        show
    };
 return {
    show: addPlayer,
    createCoverClickHandler,
    createPlayer
  };
  return {
    add: addPlayer,
    createCover: createCoverClickHandler(),
    create: createPlayer(),
    show
  };
 return {
        add: addPlayer,
        createCover: createCoverClickHandler,
        create: createPlayer,
        show
    };

Please supply a link to your updated code that you think most closely follows what Step 1 asks, and I’ll try to provide some advice in regard to that code.

The only thing I touched in the code was inside the return.
That was the only thing that was meant to be touched.

Inside the return was the only thing being worked on in Step 1.
I am not able to move on to Step 2 unless Step 1 is correct.

I also know I’m not doing something the right way because if I was, I would be able to do Step 2, and so far I have not been able to. Which means, I am still stuck on Step 1.

You keep telling me I’m doing it wrong, so I’m very confused.

Do either of these returns look right to you?

I don’t know how to write the returns a different way.

This code: https://jsfiddle.net/9qz7bujL/

return {
        add: addPlayer,
        createCoverClickHandler,
        createPlayer,
        show
    };

or this code:

This code: https://jsfiddle.net/gcfkwmLr/1/

return {
        add: addPlayer,
        createCover: createCoverClickHandler,
        create: createPlayer
        show
    };

Completing Step 1 is supposed to prepare me for Step 2.

In Step 2 I am supposed to be able to move these functions out, and I am still not able to.

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);
    }

Which one of those links to code should I put my efforts into providing advice?