2
\$\begingroup\$

When a button is pressed, an animation loop is triggered. At the start of the loop, I create a new time object.

I also have a pause button. A problem was created when paused and then started as the Date object would be recreated each time.

The variable is global and not defined to be anything. I could set it to 0 and run the conditional test on that instead, but either way there has to be a cleaner way of doing this. Apologies, I'm quite new to JS.

The code works, but it's ugly (in my opinion):

var t;//not defined

//for starting the animation
startSim.onclick = function(){

    if(t == undefined){
        //get time object
        t = new Date();
    }

    //other code...

}

EDIT: In case the question is unclear - I'm checking to see if the Date object has been created by finding out if the variable is undefined or not. What I'm asking is simply Is there a way to find out if the Date object already exists that doesn't rely on checking if the var is undefined. There is little code as the above is the only relevant part, I can post the rest but it's pointless as not required. (I've added a little more to show structure but I don't think it's required to understand the question)

\$\endgroup\$
7
  • \$\begingroup\$ You want us to review 2 lines of code? \$\endgroup\$ Jan 13, 2014 at 2:12
  • \$\begingroup\$ @GeorgeMauer - was the question unclear? \$\endgroup\$
    – null
    Jan 13, 2014 at 2:24
  • \$\begingroup\$ Yes it is, I can't tell what you're asking. Generally this is for code review meaning that you need code to be reviewed. I'm not against asking for design review but you've got to give me a design to review - a chart, some pseudocode, something \$\endgroup\$ Jan 13, 2014 at 3:13
  • \$\begingroup\$ @GeorgeMauer - apologies, I thought the question was clear, edited to try and clarify further... \$\endgroup\$
    – null
    Jan 13, 2014 at 9:06
  • 1
    \$\begingroup\$ Consider t = t || new Date() which either keeps t if it is defined or assigns the new Date() value \$\endgroup\$
    – konijn
    Jan 13, 2014 at 21:18

1 Answer 1

2
\$\begingroup\$

I can think of the following improvements:

  1. Hide t. If you're running other JS modules, you can't really be sure that no-one else writes to the same variable.
  2. If you want to be sure that your code doesn't mix up an uninitialized timer with a "cleared" timer, you might want to pre-initialize the variable to null instead.
  3. Use === when comparing null, undefined, etc. In JavaScript, null == undefined, but only null === null.

You might want to do something like this:

var timer = (function () {
    var t = null,

    start = function () {
        if (t === null) {
            t = new Date();
        }
    },

    stop = function () {
        var time = new Date() - t;
        t = null;
        return time;
    };

    return {
        start: start,
        stop: stop
    };
})();

timer.start();

var timePassed = timer.stop();
\$\endgroup\$
2
  • \$\begingroup\$ thank you for the suggestions and advice, it's much better than what I had, thank you. \$\endgroup\$
    – null
    Jan 13, 2014 at 12:03
  • \$\begingroup\$ Hi again, I'm just trying to implement a pause feature into the timer function and can't seem to get it right, would you mind lending a hand please? I've added a pause function such as: pause = function(){ pause = true; pausedTime = t; return pausedTime; }, But it's not working as expected. \$\endgroup\$
    – null
    Feb 15, 2014 at 19:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.