0

I have nested function in function of JavaScript and I want to call it in my main function photoGallery() of my html code, but didn't work. Where I'm wrong ?

JavaScript:

function photoGallery1() {

    kartinki = new Array('images/green_salad1.png', 'images/green_salad2.png', 'images/green_salad3.png');
        index = 0;

    function next() {
        index++;
        if ( index >= kartinki.length) index = 0; 
        document.getElementById('image2').src = kartinki[index];
    }

    function previous() {
        index--;
        if ( index < 0) index = kartinki.length -1; 
        document.getElementById('image2').src = kartinki[index];
    }

    function start() {
        index = 0;  
        document.getElementById('image2').src = kartinki[index];
    }
}

The HTML code:

<!Doctype html>
<html xmlns="http://www.w3.org/1999/xhtml">
    <head>
        <meta name="viewport" content="width=device-width">
        <meta charset="utf-8">

        <title>The right eating of employed people</title>

        <link rel='stylesheet' media='screen and (max-width: 1000px)' href='css/narrow.css'/>
        <link rel='stylesheet' media='screen and (min-width: 1001px) and (max-width: 1235px)' href='css/medium.css' />
        <link rel='stylesheet' media='screen and (min-width: 1236px)' href='css/wide.css' />
        <link rel="stylesheet" href="css/calendarview.css">
        <script src="js/photogallery.js"></script> 
        <script src="js/prototype.js"></script>
        <script src="js/calendarview.js"></script>
        <script type="text/javascript">
         window.onload = function() {
            Calendar.setup({
              parentElement : 'calendar'
            })
          window.onload = photoGallery1()
          }
        </script>
Santiya
  • 195
  • 3
  • 12
  • 2
    Any error in your browser console – Arun P Johny Jun 09 '16 at 13:29
  • In the browser console I have: ReferenceError: next is not defined – Santiya Jun 09 '16 at 13:32
  • What do you think `window.onload = photoGallery1()` is doing? Where are you calling `next`? – Felix Kling Jun 09 '16 at 13:32
  • @Santiya: then you call `next` somewhere and that's why crashes – István Jun 09 '16 at 13:34
  • next() isnt going to be available outside photoGallery1(). Its going to be private to that function. – terpinmd Jun 09 '16 at 13:34
  • I think when I call photoGallery1(), automaticly will call and the others funcitons - next, previous, star. How to make it public or to call them? – Santiya Jun 09 '16 at 13:37
  • @Santiya: *Defining* a function and *calling* a function are two different things. The error you are getting means that you are trying to access `next` somewhere where it is not available. However, in the code you posted, you are not referring to `next` anywhere. – Felix Kling Jun 09 '16 at 16:44

7 Answers7

1

I think this is what you were going for. That window.onload = photoGallery1() inside of the window.onload callback made no sense to me.

window.onload = function() {
  Calendar.setup({
    parentElement : 'calendar'
  });
  photoGallery1();
}

This will call the photoGallery1() function when the window.onload event fires. There are a lot of issues, however, with your script. Lots of things to improve.

mrbinky3000
  • 4,055
  • 8
  • 43
  • 54
1

First of, you are assigning the executed photoGallery1() function to window.onload, so basically the result of photoGallery1(). You need to assign the function itself:

 window.onload = photoGallery1;

In your function photoGallery1() there is no functions being executed or returned. When we refer to scope, it means where certain functions and variables is visible from.

If you look at the functions inside photoGallery1, they are inside the scope of photoGallery1 and can not be accessed or executed from the outer scope.

One possible solution would be to do:

function photoGallery1() {

    function start() {
     // do your things
    }

    // invoke function
    start();
}
window.onload = photoGallery1;

Another is to expose some of your funnctions by returning some of the functions that you need:

function photoGallery1() {
    function start() {
     // do your things
    }
    function next(){};
    function previous(){};
    return {
         start: start,
         next: next,
         previous: previous
    }
}

// Execute your functions
var photoGallery = photoGallery1();
window.onload = photoGallery.start;
Chris Malherbe
  • 238
  • 2
  • 9
1

First:

window.onload = photogallery1();

Results in an undefined value for the window.onload property.

As @mrbinky3000 said, you need to call photogallery1() in your onload event handler.

Furthermore, you need an object with public methods in order to make this accessible from an outside scope, in which case you need a Constructor Function:

function Photogallery() {
    // Don't forget the "var" directive to prevent these from being global
    var kartinki = new Array('images/green_salad1.png', 'images/green_salad2.png', 'images/green_salad3.png');
    var index = 0;

    this.next = function () {
        index++;
        if ( index >= kartinki.length) index = 0; 
        document.getElementById('image2').src = kartinki[index];
    }

    this.previous = function () {
        index--;
        if ( index < 0) index = kartinki.length -1; 
        document.getElementById('image2').src = kartinki[index];
    }

    this.start = function () {
        index = 0;  
        document.getElementById('image2').src = kartinki[index];
    }
}

Now your onload changes a little:

var photoGallery = null;

window.onload = function () {
    // the other stuff you had
    photoGallery = new Photogallery();
}

Don't forget to declare the photoGallery variable to avoid it being an implicitly declared global variable.

Now a little HTML to call the methods on your object:

<button type="button" onclick="photoGallery.next()">Next</button>
<button type="button" onclick="photoGallery.previous()">Previous</button>
Community
  • 1
  • 1
Greg Burghardt
  • 17,900
  • 9
  • 49
  • 92
1

photoGallery1() should be instantiated

var Gallery = new photoGallery1();

Functions you declared in the body of photoGallery1() are private, so you have to attach them to events inside photoGallery1.

You can look at the function as the Class and Constructor in one. So use it accordingly.

v-andrew
  • 21,939
  • 6
  • 36
  • 41
1

I'm not exactly sure what you are trying to accomplish, but if it is a photo gallery like my common sense would indicate, then these three things may help.

  1. Remove any superfluous information from your example, as it confuses the issue you are trying to solve. (f.e. the calendar.js and CSS stylesheet calls). This will allow others to help you in a more effective manner.

  2. Separate your function from your form. It is generally good practice to use HTML strictly for the skeleton of the web page/app and keep the abilities of the skeleton (the functions the page/app can do) in the javascript. This is demonstrated in my example.

  3. Instead of nesting functions try turning your "photogallery" into an object and assigning the "next", "previous", and "start" methods to the appropriate event. (In my example I assigned "next" and "previous to buttons and "start" to window.onload)

The External Javascript File: "photogallery.js"

    /*
     * Wrapping code in an iife is always good practice to prevent the pollution of the global 
     * name space.
     */
    (function(){
    /* 
     * Declare your array of images and index outside of the photoGallery1 object so the
     * methods of photoGallery1 can cleanly reference them before photoGallery1 is initialized 
     * in the global execution context
     */
    var kartinki =   ['images/green_salad1.png', 'images/green_salad2.png', 'images/green_salad3.png'];
    var index = 0;

    var photoGallery1 =  {
       next:  function (){
          index++;
          /* 
           * Here the index will never be greater than kartinki.length so check against 
           * (kartinki.lenghth - 1) or use index == kartinki.length
           */
          if (index > (kartinki.length - 1)) { index = 0 };
          document.getElementById('image2').src = kartinki[index];
        },

       previous: function() {
          index--;
          if ( index  < 0) { index = kartinki.length - 1 };
          document.getElementById('image2').src = kartinki[index];
       },

       start: function() {
          document.getElementById('image2').src = kartinki[index];
       }
    }

    /* 
     * Do the below inside an external javascript file rather than the html you can set the 
     * window object's onload property to an anonymous function, in which you can call any 
     * functions you want to happen when the page loads (i.e. photoGallery1.start() is called).
     */
    window.onload = function(){
       photoGallery1.start()
    }
    //Setting the "next" and "previous" methods to there corresponding buttons
    document.getElementById('prev').onclick = photoGallery1.previous
    document.getElementById('next').onclick = photoGallery1.next
    })()

The HTML file: "index.html"

    <!doctype html>
    <html xmlns="http://www.w3.org/1999/xhtml">
       <head>
          <title>The right eating of employed people</title>
       </head>
       <body>
          <img id="image2">
          <button id="prev">Previous</button> <button id="next">Next</button>
          <!-- Add the script tag at the bottom of the body so the browser can render the  
          html elements referenced in photogallery.js before they are needed. If you don't do
          this document.getElementById("image2") will return null, as it has not been created 
          at the time of photogallery.js's execution.
          -->
          <script src="/photogallery.js"></script>
       </body>
    </html>

If you have any questions please don't hesitate to ask! :D

Ike10
  • 1,585
  • 12
  • 14
  • Haha thank you too! :) But now I'm done with all that help I've got and now everything is alright ! :) – Santiya Jun 09 '16 at 16:39
0

If I got, you want the 3 functions inside photoGallery1() to be called only when photoGallery1() is called. If it is the point, just call them at the end before to close.

function photoGallery1() {

    kartinki = new Array('images/green_salad1.png', 'images/green_salad2.png', 'images/green_salad3.png');
        index = 0;

    function next() {
        index++;
        if ( index >= kartinki.length) index = 0; 
        document.getElementById('image2').src = kartinki[index];
    }

    function previous() {
        index--;
        if ( index < 0) index = kartinki.length -1; 
        document.getElementById('image2').src = kartinki[index];
    }

    function start() {
        index = 0;  
        document.getElementById('image2').src = kartinki[index];
    }

    next();
    previous();
    start();
}
IgorAlves
  • 5,086
  • 10
  • 52
  • 83
  • Yes you've got me right! That I want to do and I've try what you suggest and again no working. – Santiya Jun 09 '16 at 13:43
  • This is what I do for my functions and it works good. Did you try call each function separated and independent. Only to test if it works? I mean, delete the function declaration and let the code run for the 1st function. If it is ok, call it as I suggested. Do the same to the others 2 – IgorAlves Jun 09 '16 at 14:03
  • Mhmm, I try this but again it's sending me the ReferenceError: next is not defined. – Santiya Jun 09 '16 at 14:39
  • Put next outside above photoGallery1() and call it again just for test. – IgorAlves Jun 09 '16 at 14:49
0

Thank you to all for the helping and tips! :) Already is working fine and how I wanted! Lastly I post the final codes. HTML:

<!Doctype html>
    <html xmlns="http://www.w3.org/1999/xhtml">
        <head>
            <meta name="viewport" content="width=device-width">
            <meta charset="utf-8">

            <title>Правилното хранене на заетите хора</title>

            <link rel='stylesheet' media='screen and (max-width: 1000px)' href='css/narrow.css'/>
            <link rel='stylesheet' media='screen and (min-width: 1001px) and (max-width: 1235px)' href='css/medium.css' />
            <link rel='stylesheet' media='screen and (min-width: 1236px)' href='css/wide.css' />
            <link rel="stylesheet" href="css/calendarview.css">
            <script src="js/photogallery.js"></script> 
            <script src="js/prototype.js"></script>
            <script src="js/calendarview.js"></script>
            <script type="text/javascript">
              window.onload = function() {
                Calendar.setup({
                  parentElement : 'calendar'
                })
                photoGallery = new Photogallery();
              }
            </script>
           <body>
           ......
           <p id="photogallery">
           <a href="javascript:void(0)" class="prev" onclick="photoGallery.previous()"><img src="images/prev.png" border="0"></a><a href="javascript:void(0)"><img src="images/home.png" border="0" onclick="photoGallery.start()"></a><a href="javascript:void(0)" class="next" onclick="photoGallery.next()"><img src="images/next.png" border="0"></a>
           </p>
           ....
           </body>
    </html>

JavaScript code:

function Photogallery() {

    var kartinki = new Array('images/green_salad1.png', 'images/green_salad2.png', 'images/green_salad3.png');
    var index = 0;

    this.next = function () {
        index++;
        if ( index >= kartinki.length) index = 0; 
        document.getElementById('image2').src = kartinki[index];
    }

    this.previous = function () {
        index--;
        if ( index < 0) index = kartinki.length -1; 
        document.getElementById('image2').src = kartinki[index];
    }

    this.start = function () {
        index = 0;  
        document.getElementById('image2').src = kartinki[index];
    }
}

var photoGallery = null;

window.onload = function () {
    photoGallery = new Photogallery();
}
Santiya
  • 195
  • 3
  • 12