3
\$\begingroup\$

This is a jQuery time update events for a video player with multiple ids. I feel the code is too long and I need to add more ids to fire events.

 $(document).ready(function() {
           var i = 0; 
           var a = 0; 
           var b = 0; 
           var c = 0; 
           var j = 0;

           $("#r-news-week1, #r-news-week2").bind("play", function() {
                if (i < 1) {
                    console.log("Video Started");
                    ga('send', 'event', 'Videos', 'Started', 'Video');
                }
                i = i + 1;
            });

             $("#r-news-week1, #r-news-week2").bind("timeupdate", function() {
                  var currentTime = this.currentTime;
                  if (currentTime > 0.75 * (this.duration)) {
                    if (c < 1) {
                      console.log("watched 75%");
                      ga('send', 'event', 'Videos', 'Watched 75%', 'Video');
                    } c = c + 1; 
                  } else if (currentTime > 0.50 * (this.duration)) {
                      if (b < 1) {
                      console.log("watched 50%");
                      ga('send', 'event', 'Videos', 'Watched 50%', 'Video');
                    } b = b + 1;
                  } else if (currentTime > 0.25 * (this.duration)) {
                      if (a < 1) {
                      console.log("watched 25%");
                      ga('send', 'event', 'Videos', 'Watched 25%', 'Video');
                    } a = a + 1;
                  }
              });


            $("#r-news-week1, #r-news-week2").bind("ended", function() {
                if (j < 1) {
                    console.log("Finished 100%");
                    ga('send', 'event', 'Videos', 'Finished 100%', 'Video');
                }
                j = j + 1;
            });

        })
\$\endgroup\$
1
  • \$\begingroup\$ Variable names i, a, ... bind is deprecated, use on. Cache $("#r-news-week1, #r-news-week2"), Chain methods on same object. For play and ended, the if conditions are not required as these events will fire only once, you can use one() to bind such events. \$\endgroup\$ Commented Dec 20, 2016 at 8:45

1 Answer 1

Reset to default
1
\$\begingroup\$

If you can us the timeupdate event to detect start and finish events you could use a single event listener block. You could also put the events an an array and increment the current position in the video being listened for. Something similar to this:

function bindVideoEvents(ids) {
    var _events = [
        { time: 0.00, name: 'Started' },
        { time: 0.25, name: '25% Watched' },
        { time: 0.50, name: '50% Watched' },
        { time: 0.75, name: '75% Watched' },
        { time: 1.00, name: 'Finished 100%' }
    ];
    var _evPos = {};

    $.each(ids, function (idx, id) {
        _evPos[idx] = 0;
        $('#' + id).on('timeupdate', function () {
            var currentEvent = _events[_evPos[idx]];
            if (this.currentTime > (currentEvent.time * this.duration)) {
                console.log(currentEvent.name);
                ga('send', 'event', 'Videos', currentEvent.name, 'Video');
                _evPos[idx]++;
            }
        });
    });
}
\$\endgroup\$
4
  • \$\begingroup\$ How about have _events outside the function (like a const), and maybe call it something like PROGRESS_PHASES? \$\endgroup\$ Commented Dec 23, 2016 at 12:25
  • \$\begingroup\$ I don't think there a reason to move them outside the function unless they are needed elsewhere. I like PROGRESS_PHASES or PHASES better for a name. I was having trouble coming up with a good name for that constant. \$\endgroup\$ Commented Dec 23, 2016 at 13:06
  • \$\begingroup\$ The reason to move outside the function is that you could want to configure your phases, and more you don't have to create the variable each time you call the function. \$\endgroup\$ Commented Dec 23, 2016 at 13:15
  • \$\begingroup\$ It depends on how many times he function needs called. If it only is called once then there is no inefficiency. If the phases are not needed outside the function I'm find with the negligible performance hit to make the code more readable by keeping all the phases and events in the same location. It it were necessary to configure the phases or call the method a large number of time I would the consider moving them out. Until them I prefer to follow YANGI. \$\endgroup\$ Commented Dec 23, 2016 at 18:38

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.