11

Here's a quick (broke) jsfiddle: http://jsfiddle.net/wH2qF/

This isn't working for some reason... is it because I have a setTimeout inside a handler of another setTimeout?

$(function() {
   $("#Volume").click(function() {
      setTimeout(triggerVolumeChange, 4000);
      function triggerVolumeChange() 
      {
          var volumeDiv = document.getElementById("volumeNumber");
          var volumeOld = 8;
          var volumeNew = 37;
          var timeNew = (1000/(volumeNew-volumeOld));
          changeVolume();
                        
          function changeVolume()
          {
             volumeDiv.innerHTML = volumeOld;
             volumeOld++;
             if (volumeOld <= volumeNew) setTimeout(changeVolume, timeNew);
          };
      });
});

Should specify that for clarity purposes I deleted other things from that Click function, and also to clarify what doesn't work exactly, well, basically, I click and nothing happens, whereas if I cut out this chunk of code it works fine... actually the setting of the vars also work fine (naturally I presume) but when I paste or uncomment the changeVolume() function then the click stops working again... Any thoughts?

--

Another piece of clarification: What I'm trying to do is, on click, simulate the volume going from value 8 to 37, in a string.. thus the setTimeout inside that function.

--

As per your guy's request, here's the entire code... I doubt it will make sense, but here it is... FYI, on click this will trigger a number of animations to simulate the flow of an application I'm designing..

<script>
            $(function() {
                $("#Volume").click(function() {
                                        
                    var userPrompt = document.getElementById("userPrompt")
                    userPrompt.innerHTML = "Change volume to 37";
                                        
                    var avatarIcon = document.getElementById("avatarIcon");
                    avatarIcon.innerHTML = "<img src='imgs/haloIcons-volume_82x76.png' alt='Volume'/>";
                
                    var hints = document.getElementById("hints");
                    hints.style.opacity = 0;
                    $(".dragonTvOut").toggleClass("dragonTvIn");
                    
                    setTimeout(triggerP, 1000);
                    function triggerP()
                    {
                        var halo = document.getElementById('avatar');
                        if( 'process' in halo ) { 
                            halo.process();
                        };
                    };
                    
                    setTimeout(triggerUserPrompt, 2000);
                    function triggerUserPrompt() 
                    {
                        document.getElementById("userPrompt").className = "userPromptIn";
                    };
                    
                    setTimeout(triggerVolumeChange, 4000);
                    function triggerVolumeChange() 
                    {
                        document.getElementById("userPrompt").className = "userPromptEnd";
                        
                        var halo = document.getElementById('avatar');
                        if( 'resume' in halo ) { 
                            halo.resume();
                        }

                        document.getElementById("avatarIcon").className = "avatarIconEnd";
                        
                        var volumeDiv = document.getElementById("volumeNumber");
                        var volumeOld = 8;
                        var volumeNew = 37;
                        var timeNew = (1000/(volumeNew-volumeOld));
                        changeVolume();
                        
                        function changeVolume()
                        {
                            volumeDiv.innerHTML = volumeOld;
                            volumeOld++;
                            if (volumeOld <= volumeNew) setTimeout(changeVolume, timeNew);
                        }​;
                        
                        var side = 100;
                        var paper = new Raphael(volumeArcAnim, 100, 300);
                        
                        paper.customAttributes.arc = function (xloc, yloc, value, total, R) {

                            var alpha = 360 / total * value,
                                a = (90 - alpha) * Math.PI / 180,
                                x = xloc + R * Math.cos(a),
                                y = yloc - R * Math.sin(a),
                                path;
                            if (total == value) {
                                path = [
                                    ["M", xloc, yloc - R],
                                    ["A", R, R, 0, 1, 1, xloc - 0.01, yloc - R]
                                ];
                            } else {
                                path = [
                                    ["M", xloc, yloc - R],
                                    ["A", R, R, 0, +(alpha > 180), 1, x, y]
                                ];
                            }
                            return {
                                path: path
                            };
                        };
                    
                        var arcWidth = 87;
                        var strokeRadius = arcWidth/2;
                        
                        var indicatorArc = paper.path().attr({
                            "stroke": "#ffffff",
                            "stroke-width": 3,
                            arc: [side/2, side/2, 12, 100, strokeRadius]
                        });
                        
                        indicatorArc.animate({
                            arc: [side/2, side/2, 60, 100, strokeRadius]
                        }, 1500, "<>", function(){
                            // anim complete here
                        });
                        
                    };
                                            
                });
            });
            </script>
oklas
  • 7,935
  • 2
  • 26
  • 42
ttothec
  • 323
  • 1
  • 3
  • 10
  • What errors are you getting? I think `setTimeout()` takes a string for the method call. – knownasilya Dec 14 '12 at 22:34
  • @Knownasilya - It takes a string or a function. – James Gaunt Dec 14 '12 at 22:35
  • 2
    @Knownasilya [never pass strings to setTimeout](http://stackoverflow.com/questions/797115/settimeout-how-to-avoid-using-string-for-callback). – jbabey Dec 14 '12 at 22:35
  • Have you checked the console for errors? – knownasilya Dec 14 '12 at 22:36
  • 1
    you might want to consider refactoring your functions in such a way that they pass parameters to each other rather than relying on executing in the parent function's scope. – jbabey Dec 14 '12 at 22:37
  • It seems most likely the code just contains a syntax error. If the code above isn't the actual code can you paste the actual code so we can check it? Alternatively just use the browsers debugging tool to step through it and see where it goes wrong. – James Gaunt Dec 14 '12 at 22:39
  • but it works fine here http://jsfiddle.net/JzyYw/2/ – ttothec Dec 14 '12 at 22:39
  • @jbabey because it's deprecated? At least that's what I get from MDN. – knownasilya Dec 14 '12 at 22:39
  • 2
    @Knownasilya a few reasons: it has to `eval` your string, which is known to be a bad practice for its own reasons. also, it forces your "code" (string) to be executed in the global context which, in examples such as the OPs code, would break the code (neither `triggerVolumeChange` nor `changeVolume` would be defined in the global context) – jbabey Dec 14 '12 at 22:41
  • pasted the entire code above... – ttothec Dec 14 '12 at 22:41
  • does having the entire code above help? does anyone find any red flags? – ttothec Dec 14 '12 at 22:44
  • @user1810062 It will be more helpful to create a working (broken) example in http://jsfiddle.net – Ruan Mendes Dec 14 '12 at 22:45
  • Juan Mendes, here it is... http://jsfiddle.net/wH2qF/ – ttothec Dec 14 '12 at 22:51
  • just to compare... this works: http://jsfiddle.net/7rGSb/ this does not http://jsfiddle.net/wH2qF/ – ttothec Dec 14 '12 at 23:05
  • @ttothec Please see my answer and comment on it, the jsfiddle you added pretty much works – Ruan Mendes Dec 14 '12 at 23:30

5 Answers5

5

Yes, you can have a setTimeout() inside another one -- this is the typical mechanism used for repeating timed events.

The reason yours isn't working is not to do with the setTimeout() itself; it's to do with the way you've nested the functions.

The changeVolume() function being inside triggerVolumeChange() means that you can't reference it directly using its name.

The quickest solution for you would be to remove the nesting, so that changeVolume() is at the root level rather than nested inside triggerVolumeChange().

Spudley
  • 166,037
  • 39
  • 233
  • 307
  • 1
    Your comment about it being actually `triggerVolumeChange.changeVolume` is not correct. It is true that `changeVolume` is only accessible inside `triggerVolumeChange` so it's likely the problem here. I'd upvote this if you fixed/clarified that part – Ruan Mendes Dec 14 '12 at 22:40
  • @JuanMendes - Sure, I'll amend it. I know it can't be called that way (hence that wasn't my advice); I was trying to point out how it fitted into the namespace the way it was written rather than how it would be called. – Spudley Dec 14 '12 at 22:43
  • Any ideas on how to "convert" that function to work in the root of `triggerVolumeChange()` – ttothec Dec 14 '12 at 22:48
  • Hmmm... When moving `changeVolume()` out of `triggerVolumeChange()` also makes all variables used in `changeVolume()` undefined, since they will be out of the scope... – Teemu Dec 14 '12 at 22:48
  • mmm... actually, `changeVolume` is not referenced outside of `triggerVolumeChange`? – Ruan Mendes Dec 14 '12 at 22:49
  • @JuanMendes Not as it is in the post, but according Spudley's answer, it should be taken away from the original scope... – Teemu Dec 14 '12 at 22:52
  • Hey guys I went ahead and put this on a jsfiddle... any help? – ttothec Dec 14 '12 at 23:02
4

You're missing an }:

$(function() {
   $("#Volume").click(function() {
      setTimeout(triggerVolumeChange, 4000);
      function triggerVolumeChange()
      {
          var volumeDiv = document.getElementById("volumeNumber");
          var volumeOld = 8;
          var volumeNew = 37;
          var timeNew = (1000/(volumeNew-volumeOld));
          changeVolume();

          function changeVolume()
          {
             volumeDiv.innerHTML = volumeOld;
             volumeOld++;
                 if (volumeOld <= volumeNew) setTimeout(changeVolume, timeNew);
          };
      } // that one was missing
   });
});
Sirko
  • 72,589
  • 19
  • 149
  • 183
3

In your broken example http://jsfiddle.net/wH2qF/ there are a few problems

  • You forgot to tell jsfiddle to use jQuery
  • The id of the volume span (in JS) was ph but should be volumeNumber (as in the HTML)

Click here to see a working version

Had you selected jQuery from the libraries in jsfiddle, you would have seen an error

Uncaught TypeError: Cannot set property 'innerHTML' of null

That leads me to believe that your jsfiddle is not a good representation of your problem. Maybe try to create another reduction since the one you added only had "silly" errors?

Ruan Mendes
  • 90,375
  • 31
  • 153
  • 217
0

If you don't want to use setInterval(), you can make the code work with these changes:

$(function() {
    $("#Volume").click(function() {
        setTimeout(triggerVolumeChange, 4000);
        function triggerVolumeChange () {
            var volumeDiv = document.getElementById("volumeNumber");
            var volumeOld = 8;
            var volumeNew = 37;
            var timeNew = (1000/(volumeNew-volumeOld));
            var changeVolume = function () {
                volumeDiv.innerHTML = volumeOld;
                volumeOld++;
                if (volumeOld <= volumeNew) setTimeout(changeVolume, timeNew);
            };
            changeVolume();
        }
    });
});

Working demo at jsFiddle.

Teemu
  • 22,918
  • 7
  • 53
  • 106
  • I've copied your code to a jsfiddle and it's not working... http://jsfiddle.net/wH2qF/2/ any ideas as to why? – ttothec Dec 14 '12 at 23:11
  • @ttothec I've no clue... I tested the code locally before posting it, and it worked well :(. – Teemu Dec 14 '12 at 23:25
  • @ttothec OK, it works now, just select `jQuery` from Framework panel in your fiddle. Thanks to Juan Mendez for a tip : ). – Teemu Dec 14 '12 at 23:31
0

Technically there is no difference where the timer is initiated from. In most cases it is implemented as list of the timers with an identifiers and associated callback handlers.

So it will be ok if your logic is correct. There is no unpredictable conditions that bring infinite call sequences, and there is no infinite amount of timeout instances and so on.

For example imitation of setInterval function:

// Bad (unexpected multiple sequences started per each event)

const handler = () => {
  setTimeout(handler)
}

<Button onClick={handler} />
// Good (previous interval closed before start new one)

const [id, idUpdate] = useState(-1)

const handler = () => {
  const id = setTimeout(handler)
  idUpdate(id)
}

const start = () => {
  if(id !===-1) clearTimeout(id)
  idUpdate(-1)
  handler()
}

<Button onClick={start} />

or some else

// Bad (infinite events with small time will take all cpu time)

const handler = () => {
  setTimeout(handler, 0) // actually mean several ms
}
// Good (special condition for finish iterations)

let count = 20

const handler = () => {
  if(!count) return
  count--
  setTimeout(handler, 0)
}
oklas
  • 7,935
  • 2
  • 26
  • 42