3

This is more a question involving why one of my solutions works and the other doesn't. I'm fairly new to JS, only been learning a couple of months and whilst I have most of the basics down, I feel my knowledge of best practice is lacking.

I'm creating an animated hero image for the top of an infographic and on it I'm using JS to create two trains moving across the screen, one from left to right and the other right to left. I created the following code, which worked:

    $(document).ready(function() {

        var anim = {
            train1: $(".train-one"),
            train2: $(".train-two"),


            moveLeft: function(percent, duration) {
                 anim.train1.animate({left: percent}, duration, "linear")
            },

            moveRight: function(percent, duration) {
                 anim.train2.animate({right: percent}, duration, "linear")
            },


            leftTrain: function() {
                anim.moveLeft("33%", 1000, anim.moveLeft)
                    anim.moveLeft("66%", 2000, anim.moveLeft)
                         anim.moveLeft("100%", 1000, anim.moveLeft)
                            anim.moveLeft("-100px", 1)
            },

            rightTrain: function() {
                 anim.moveRight("40%", 1000, anim.moveRight)
                    anim.moveRight("60%", 2000, anim.moveRight)
                        anim.moveRight("100%", 1000, anim.moveRight)
                                anim.moveRight("-100px", 1)
            },
        };

        anim.leftTrain();
        anim.rightTrain();
        setInterval(anim.leftTrain, 5000);
        setInterval(anim.rightTrain, 6000);

    });

What I'm wondering is why the following didn't work when I expected it to, I created a seperate method to reset the train once all the callbacks had been completed:

        resetLeftTrain: function(test) {
        anim.train1.css("left", "-100px ");
    },

    leftTrain: function() {
        anim.moveLeft("33%", 1000, anim.moveLeft)
            anim.moveLeft("66%", 2000, anim.moveLeft)
                anim.moveLeft("100%", 1000, anim.resetLeftTrain)
                        anim.resetLeftTrain()
    },

Sorry if the answer is really obvious, I'm not so used to callbacks in plain JS. Please feel free to give any other pointers regarding structure etc. Really appreciate it!

Cheers.

EDIT: I solved the issues thanks to the answers below and the code now works perfectly as follows:

    $(document).ready(function() {

        var anim = {
            train1: $(".train-one"),
            train2: $(".train-two"),


        moveLeft: function(percent, duration, callback) {
            this.train1.animate({left: percent}, duration, "linear", callback)
        },

        moveRight: function(percent, duration, callback) {
            this.train2.animate({right: percent}, duration, "linear", callback)
        },

        resetLeftTrain: function() {
            this.train1.css("left", "-100px");
        },

        resetRightTrain: function() {
            this.train1.css("right", "-100px");
        },

        leftTrain: function() {
            var self = this;

            this.moveLeft("33%", 1000, function() {
                self.moveLeft("66%", 2000, function(){
                    self.moveLeft("100%", 1000, function(){
                        self.resetLeftTrain();
                    });
                });
            });
        },

        rightTrain: function() {
            var self = this;

            this.moveRight("40%", 1000, function() {
                self.moveRight("60%", 2000, function(){
                    self.moveRight("100%", 1000, function(){
                        self.resetRightTrain();;
                    });
                });
            });
        },
    };

    anim.leftTrain();
    anim.rightTrain();
    setInterval($.proxy(anim.leftTrain, anim), 5000);
    setInterval($.proxy(anim.rightTrain, anim), 6000);

    });

Next time I may look into using the Jquery .promise() method to avoid too much ugly indentation.

Thanks for all the help, hope the question and it's answers are useful to others

  • NB: referring an an object (your `anim`) by name from within methods declared _on that object_ is usually a _really bad idea_. It breaks encapsulation. – Alnitak Sep 19 '14 at 11:08
  • 1
    Also, your class should really only (IMHO) handle _one_ train, with separate methods for moving it left or right. There would then be two separate instances of that class. – Alnitak Sep 19 '14 at 11:09
  • Thanks Alnitak, what would be the correct way to refer to the object from methods within itself? – user3586963 Sep 19 '14 at 11:42
  • usually as `this`, although there are minor complications relating to passing object methods around as callbacks and ensuring that `this` refers either to an element or to the wrapping object. – Alnitak Sep 19 '14 at 13:02
  • Thanks man, I solved the referencing issues as you can see in the edit above, appreciate the help. – user3586963 Sep 19 '14 at 13:28
  • @Alnitak: Not in this case, where `anim` is in a controlled scope. [Both that solution and using `this` have their issues](http://stackoverflow.com/a/10711164/1048572). – Bergi Sep 19 '14 at 14:27

2 Answers2

0

You need to provide anonymous callback functions to the animations. Your lack or semi-colons give the impression they are nested :)

 anim.moveRight("40%", 1000, function(){
    anim.moveRight("60%", 2000, function(){
        anim.moveRight("100%", 1000, function(){
                anim.moveRight("-100px", 1);
        });
     });
 });

and make your methods take a callback to pass on:

moveRight: function(percent, duration, callback) {
     anim.train2.animate({right: percent}, duration, "linear", callback);
},

The end result is that as each animate call finished, it will execute the code provided, effectively chaining the animations together.

iCollect.it Ltd
  • 92,391
  • 25
  • 181
  • 202
  • Given the tick because it came through first, this is bizarrely almost exactly what I started with, however I didn't declare the callback as a parameter within the moveRight method, hence why it stopped running! I'm obviously so used to JQuery methods automatically accepting callback arguments. – user3586963 Sep 19 '14 at 10:59
  • Quick question though - is "anim.moveRight" really required before the callbacks? – user3586963 Sep 19 '14 at 11:00
  • Argh... No. Cut & paste errors account for 95% of all my bugs nowadays :) – iCollect.it Ltd Sep 19 '14 at 11:01
0

Year I also needed a few weeks to understand the "callback"-concept right ;-)

Here are an example (only for your left train):

$(document).ready(function() {

    var anim = {

        train1: $(".train-one"),

        moveLeft: function(percent, duration, cb) {
            anim.train1.animate({left: percent}, duration, cb);
        },

        leftTrain: function() {
            anim.moveLeft("33%", 1000, function() {
                anim.moveLeft("66%", 2000, function(){
                    anim.moveLeft("100%", 1000, function(){
                        anim.moveLeft("-100px", 1);
                    });
                });
            });

        },
    };

    anim.leftTrain();
    setInterval(anim.leftTrain, 5000);
});

Things you should look at:

  • You need to add callback-calls (here as a param) to your functions
  • You missed a few ";"
  • If you have trouble with all the "callback-hell" look for "promises" (jQuery has a build-in function for that) or "async.js" <- i really love that
mdunisch
  • 3,627
  • 5
  • 25
  • 41