0

Whenever I run this code all the variables and functions are "not defined" it says, but I see no errors

This is my code, thanks

    <script>
        function Random() {
            var rand = Math.floor(Math.random() * 600);
        }
        $(function() {
            function Game() {
                $("#btn_1", "#btn_2", "#btn_3").fadeIn();
                Random()
                $("#btn_1").css("margin-left", rand);
                Random()
                $("#btn_1").css("margin-top", rand);
                Random()
                $("#btn_2").css("margin-left", rand);
                Random()
                $("#btn_2").css("margin-top", rand);
                Random()
                $("#btn_3").css("margin-left", rand);
                Random()
                $("#btn_3").css("margin-top", rand);
            }
        });

    </script>
</head>
<body>
    <button id="start">Start</button>
    <button id="btn_1" class="btn">One</button>
    <button id="btn_2" class="btn">Two</button>
    <button id="btn_3" class="btn">Three</button>
</body>
Sam
  • 41
  • 1
  • 7
  • 3
    `rand` is in the scope of `Random()` and therefore not accessible in `Game()`. – univerio Jun 12 '14 at 22:35
  • 1
    All the variables and functions are not defined? The only variable I can see with a problem is `rand`, because it's local to the `Random()` function. – Barmar Jun 12 '14 at 22:35
  • when do you call Game() ? you're not telling it to do anything... – dandavis Jun 12 '14 at 22:36
  • Have you loaded jQuery? If you didn't, that would cause more errors. – Barmar Jun 12 '14 at 22:36
  • I type Game() in console. Yes, I have jQuery loaded – Sam Jun 12 '14 at 22:37
  • Just have Random return the value instead and then use `$("#btn_1").css("margin-left", Random());` but otherwise your issue is what @univerio said – GillesC Jun 12 '14 at 22:37
  • @Sam: there is no Game from the console, it's a private variable in an anon ready() function, no way to reach it from the outside... – dandavis Jun 12 '14 at 22:38
  • In general you don't need to define functions inside the document.ready function unless you want to HIDE them from the global scope. – Barmar Jun 12 '14 at 22:45

2 Answers2

0

As has a already been said, your rand variable is a local variable and thus not available outside its function. But, rather than declare it as a global, you can just return the random value from your function and use the return value of the function and you can also chain some of your operations like this and I see no reason why any of this code is inside a jQuery ready block:

<script>
    function Game() {

        function rand() {
            return Math.floor(Math.random() * 600);
        }

        $("#btn_1, #btn_2, #btn_3").each(function() {
            $(this).css({"margin-left": rand(), "margin-top": rand()}).fadeIn();
        });
    }

</script>
</head>
<body>
    <button id="start">Start</button>
    <button id="btn_1" class="btn">One</button>
    <button id="btn_2" class="btn">Two</button>
    <button id="btn_3" class="btn">Three</button>
</body>

This has these benefits:

  1. Removes the need for a global rand variable
  2. Removes the use of side effects
  3. Reduces the number of times you evaluate your selectors and create jQuery objects
  4. Makes your Game function available in the global scope so you can call it anywhere (which it sounds like you think you need).
  5. Removes the jQuery ready block which is not needed, but keeps the Random() function in a private context so it doesn't pollute the global namespace.
  6. Fixes the multiple selector to be one string with each sub-piece separated by commas (which is the correct syntax for a multi-selector).
  7. Evaluates the multi-selector only once and uses those results for all the operations.
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • If optimization was the issue then surely `$().css({'margin-top: Random(), 'margin-left': Random()});` is better than `$("#btn_1").css("margin-left", Random()).css("margin-top", Random());` And as btn_1, btn_2 and btn_3 were already queried once that could have been transverse instead of doing fresh query for each afterwards. – GillesC Jun 12 '14 at 22:58
  • @Sam - Added further optimizations/simplifications to the code in my answer which shortens it and [DRYs](http://en.wikipedia.org/wiki/Don't_repeat_yourself) it.. – jfriend00 Jun 12 '14 at 23:33
-1

Here that should work. Your problem is scope. What is declared inside a function using var is not accessible to other functions (unless they are define inside the same scope of course)

Here is how you could do this and expose Game. You could expose Random the same way.

However it is best to keep your code inside a scope or closure (function() { // your code })(); and expose only the necessary part of your code.

Here is a good read about scope: http://www.smashingmagazine.com/2009/08/01/what-you-need-to-know-about-javascript-scope/

$(function() {

    function Random() {
        return Math.floor(Math.random() * 600);
    }

    function Game() {
        $("#btn_1", "#btn_2", "#btn_3").fadeIn();
        $("#btn_1").css("margin-left", Random());
        $("#btn_1").css("margin-top", Random());
        $("#btn_2").css("margin-left", Random());
        $("#btn_2").css("margin-top", Random());
        $("#btn_3").css("margin-left", Random());
        $("#btn_3").css("margin-top", Random());
    }
    window.Game = Game;
});
GillesC
  • 10,647
  • 3
  • 40
  • 55
  • Works, only problem is I don't want all of them to have the same margins – Sam Jun 12 '14 at 22:48
  • That's another issue though, here the value generated by `Random` get assigned like you were doing with `rand` variable. But your `Random` method is not great. See http://stackoverflow.com/questions/1527803/generating-random-numbers-in-javascript-in-a-specific-range for information about generating random number – GillesC Jun 12 '14 at 22:51