0

I'm trying to give my jQuery a bit more a structure with a basic object literal pattern and encountered an issue with calling my custom move function as per the following.

(function() {

  var movingMenu = {
    menu: $('.nav').children('li'),

    move: function() {
      menu.each(function() {
        $(this).animate({
            top: '+=50'
        }, 200);
      });   
    }
  };  

})();

window.setInterval(function(){
  movingMenu.move();
}, 1000);

I'm trying to call this function every second but the calling part itself doesn't seem to work. I suspect that the variable movingMenu is maybe outside the scope that within window.setInterval it doesn't have a clue in which object this function belongs to?

Demo is available JSFiddle

Seong Lee
  • 10,314
  • 25
  • 68
  • 106
  • 2
    Your code above is different than the fiddle (hiding movingMenu within the closure). In the fiddle your reference to menu should be this.menu. – Bas Slagter Aug 31 '15 at 14:16
  • Not sure how it's different. I just removed self-invoking function as it's already provided in JSFiddle. – Seong Lee Aug 31 '15 at 14:20
  • jQuery doesn't provide a self-invoking function, it provides a function expression to create an event handler. Anyway, that makes the fiddle code different because the `setInterval` call is inside that function. – Guffa Aug 31 '15 at 14:23
  • I meant that I've already set OnLoad in the fiddle so I removed page load function in the fiddle. But I realise the difference now, I'm not wrapping `setInterval` in page load function in my post here. Thanks for pointing out. – Seong Lee Aug 31 '15 at 14:28
  • @SeongLee: Note that a self-invoking function is not the same thing as a page load function. To get a page load function you would use `$(function(){ ... })`, which is the shorthand form of `$(document).ready(function(){ ... })`. – Guffa Aug 31 '15 at 15:16
  • @Guffa Thanks. I didn't know that and it led me to this article. http://stackoverflow.com/questions/3259496/jquery-document-ready-vs-self-calling-anonymous-function – Seong Lee Sep 01 '15 at 05:09

4 Answers4

5
  1. The code you post here would not work, as you use an IIFE to wrap the object, the setInterval can't access movingMenu. However, your code in jsfiddle is correct. You can either dewrap the IIFE, or put the setInterval into the IIFE, or return a object that exposed the move function. You just need to ensure that movingMenu, or the move is accessible to setInterval.

  2. Use this to get the ref of that menu in your function, as its an attribute of movingMenu, not an variable.

Altered jsfiddle

Move everything out of IIFE:

var movingMenu = {
    menu: $('.nav').children('li'),

    move: function () {
    //  VVVV you need to use this to reference to `movingMenu`, so this.menu is the referenced `li`s.
        this.menu.each(function () {
            $(this).animate({
                top: '+=50'
            }, 200);
        });
    }
};

window.setInterval(function () {
    movingMenu.move();
}, 1000);

Move setInterval into IIFE as well:

(function(){
    var movingMenu = {
        menu: $('.nav').children('li'),

        move: function () {
        //  VVVV you need to use this to reference to `movingMenu`, so     this.menu is the referenced `li`s.
            this.menu.each(function () {
                $(this).animate({
                    top: '+=50'
                }, 200);
            });
        }
    };

    window.setInterval(function () {
        movingMenu.move();
    }, 1000);
})();
fuyushimoya
  • 9,715
  • 3
  • 27
  • 34
  • Although this gives you access to the `movingMenu` variable, this probably breaks the intended pattern. – Eric Lease Aug 31 '15 at 15:14
  • @EricLease what's the intended pattern you mean? – fuyushimoya Aug 31 '15 at 15:25
  • Typically when you wrap an object in another function it is to create a closure, resulting in private members. The fact that this object was already wrapped in a self invoking anonymous function, and should *probably* have been the return value of said function is a higher level concept than just pulling `movingMenu` out into the global scope. Admittedly, the OP should probably be dealing with the simpler concepts, before dealing with the complexity of `(function() { return publicInterface; })()` – Eric Lease Aug 31 '15 at 15:40
  • You should see comments at question, that he wraps it just because he wants to simulate the `window.onload`. And what this answer comes out is based on jsfiddle. – fuyushimoya Aug 31 '15 at 15:41
  • woops, I was trying to answer the question he asked on SO, in the context of the question he asked on SO, and ignoring the seemingly similar but unrelated question linked to in the fiddle and apparently discussed comments. – Eric Lease Aug 31 '15 at 15:55
2

Yes, you are right, the variable movingMenu is out of scope.

Also, to use the property menu inside the method you need to use this.menu. There is no object scope in JavaScript, so even if you are "inside" the object, you can't directly access the object properties.

(function() {

  var movingMenu = {
    menu: $('.nav').children('li'),

    move: function() {
      // use "this" to access properties
      this.menu.each(function() {
        $(this).animate({
            top: '+=50'
        }, 200);
      });   
    }
  };  

  // use the variable inside the scope
  window.setInterval(function(){
    movingMenu.move();
  }, 1000);

})();
Guffa
  • 687,336
  • 108
  • 737
  • 1,005
1

You're correct about movingMenu being unavailable. To get around this, you want to set your module to a variable and return whatever you want to access outside of the module.

var movingMenu = (function() {

  return {
    menu: $('.nav').children('li'),

    move: function() {
      this.menu.each(function() {
        $(this).animate({
            top: '+=50'
        }, 200);
      });   
    }
  };

})();

window.setInterval(function(){
  movingMenu.move();
}, 1000);

edit: whoops, I read object literal pattern then saw the module pattern and ran in the wrong direction.

bcr
  • 3,791
  • 18
  • 28
-1

The scope of the movingMenu object is limited to the anonymous function that wraps it... function() { var movingMenu = { ... } }.

You're using a syntax that is allowing you to define that anonymous function, and then invoke or call it immediately.

Declaration: (function() { var movingMenu = {..} })

Invocation: (function() { ... })()

This would be the same as if you said...

var foo = function() { var movingMenu = {...} };
foo();

So, in that context, movingMenu is a variable that is defined inside another function, foo. Nothing oustide of foo knows anything about movingMenu. This is the idea of scope. movingMenu exists in the scope of foo.

So to get the functionality of movingMenu outside the scope of foo, we can return movingMenu from foo. What this does is makes the return value of foo the movingMenu object, like so...

var foo = function() { 
    var movingMenu = {
        menu: $('.nav').children('li'),

        move: function() {
          menu.each(function() {
            $(this).animate({
               top: '+=50'
            },200);
          });   
        }
      };

      return movingMenu;  

    };

var menuHandler = foo(); // save movingMenu, returned from foo()

Then, you can use menuHandler like you would movingMenu.

window.setInterval(function () {
   menuHandler.move();
}, 1000);

Since you're declaring the function anonymously (not giving it a name, like I did with foo), and then you're invoking it right away, you want to store the return value right away, since you won't be able to invoke that method again.

var foo = function() { 
    var movingMenu = {
        menu: $('.nav').children('li'),

        move: function() {
          menu.each(function() {
            $(this).animate({
               top: '+=50'
            },200);
          });   
        }
      };

      return movingMenu;  

    };

var menuHandler = foo(); // save movingMenu, returned from foo()

Then, you can use menuHandler like you were trying to use movingMenu. Really, could use the name movingMenu instead of menuHandler, I just think it's less confusing this way.

window.setInterval(function () {
   menuHandler.move();
}, 1000);

So putting it altogether, with the anonymous function...

var menuHandler = (function() { 
    var movingMenu = {
        menu: $('.nav').children('li'),

        move: function() {
          menu.each(function() {
            $(this).animate({
               top: '+=50'
            },200);
          });   
        }
      };

      return movingMenu;  

    })();

window.setInterval(function () {
   menuHandler.move();
}, 1000);

So why would you do this? Why not just make movingMenu public and invoke it directly, instead of wrapping it in an anonymous method and exposing it as a return value from that method? The reason again has to do with scope. By limiting the scope and controlling what is exposed, you can actually create (somewhat, details are beyond the scope of this question) private properties in js.

For example...

var menuHandler = (function() {
    // ADDED NEXT LINE FOR EXAMPLE:
    var privateCounter = 0; 
    var movingMenu = {
        menu: $('.nav').children('li'),

        move: function() {
          menu.each(function() {
            $(this).animate({
               top: '+=50'
            },200);

            // ADDED NEXT LINE FOR EXAMPLE:
            console.log("Count is now: " + (privateCounter++).toString());
            // look at the console tab of browser's dev tools (F-12 on windows)
          });   
        }
      };

      return movingMenu;  

    })();

From this example, you can now see that movingMenu is exposed (as menuHandler), and it has the ability to use the private variable privateCounter, however, privateCounter is not exposed. So basically this pattern makes everything private initially, so you can expose just what you want to be public.

var menuHandler = (function() {
  var privateCounter = 0,
    menu = (function() {
      return $('.nav').children('li');
    })();

  var movingMenu = {
    move: function() {
      menu.each(function() {
        $(this).animate({
          top: '+=50'
        }, 200);
      });

      console.log("Count is now: " + (privateCounter++).toString());
    }
  };

  return movingMenu;
})();

setInterval(function() {
  menuHandler.move();
}, 1000);
.nav {
  position: absolute;
}
.nav li {
  position: relative;
  top: 0;
}
ul {
  list-style: none;
}
ul li {
  display: inline-block;
  background: red;
  border-radius: 50%;
  color: white;
  width: 50px;
  height: 50px;
  text-align: center;
  line-height: 3;
}
<ul class="nav">
  <li>Look</li>
  <li>Play</li>
  <li>Eat</li>
  <li>See</li>
</ul>

NOTE In my snippet I've modified your code to make the menu property non-static. This will handle items being added or removed from the menu.

Eric Lease
  • 4,114
  • 1
  • 29
  • 45
  • 1. Avoid adding a stack snippet that doses't work. Otherwise it's nonsense to include `html` and `css` part as well. 2. _This will handle items being added or removed from the menu._ No it won't, see [jsfiddle](https://jsfiddle.net/mfL1jvyx/). If you want to keep it dynamic, you should get it in `move` function instead of use IIFE to get it. – fuyushimoya Aug 31 '15 at 17:44