Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I got this code working with Jquery Countdown Plugin to take a string input and output a countdown in a div. Is there is a better way of implementing this?

JSFiddler: http://jsfiddle.net/RN2zj/2/

HTML

<div id="defaultCountdown"  class="arg">Here</div>

Script

function stringToCountdown(element,str){
        /* Parse the String */
        var hh = parseInt(str.substr(0,2),10);
        var mm = parseInt(str.substr(2,2),10);
        var ss = parseInt(str.substr(4,2),10);
        var parity = parseInt(str.substr(4,2),10);


        var austDay = new Date();
        austDay = new Date( austDay.getFullYear() , austDay.getMonth(), 
                            austDay.getDate(),austDay.getHours()+hh, 
                            austDay.getMinutes()+mm, austDay.getSeconds()+ss );     
        $('#defaultCountdown').countdown({until:austDay , compact: true,format: 'dHMs',expiryText: 'No More Counter for you'});
        }

        $(function () { 
          stringToCountdown('#defaultCountdown6','000005');
       });

Another question is if I should rewrite this to make it simpler and faster? Thank you in advance for any help

share|improve this question
add comment

1 Answer

You may combine var statements and put them top of your function. Strip unused variables like parity and use element variable instead of using #defaultCountdown directly. And lastly, a little formatting will make your code much readable. Here is the result.

If you're trying to accomplish relative countdown, plugin itself supports it. I also put an example to show how to do it.

function stringToCountdown(selector, str){
    var hh = parseInt(str.substr(0,2), 10),
        mm = parseInt(str.substr(2,2), 10),
        ss = parseInt(str.substr(4,2), 10),
        austDay = new Date();

    austDay = new Date(
        austDay.getFullYear(),
        austDay.getMonth(),
        austDay.getDate(),
        austDay.getHours() + hh,
        austDay.getMinutes() + mm,
        austDay.getSeconds() + ss
    );

    $(selector).countdown({
        until: austDay,
        compact: true,
        format: 'dHMs',
        expiryText: 'No More Counter for you'
    });
}

$(function () { 
    stringToCountdown('#defaultCountdown','000010');
});

And this is less readable version. There is no extra variable. So it may run a little bit faster.

function stringToCountdown(selector, str){
    var austDay = new Date();
    $(selector).countdown({
        until: new Date(
            austDay.getFullYear(),
            austDay.getMonth(),
            austDay.getDate(),
            austDay.getHours() + parseInt(str.substr(0,2), 10),
            austDay.getMinutes() + parseInt(str.substr(2,2), 10),
            austDay.getSeconds() + parseInt(str.substr(4,2), 10)
        ),
        compact: true,
        format: 'dHMs',
        expiryText: 'No More Counter for you'
    });
}

$(function () { 
    stringToCountdown('#defaultCountdown','000010');
});
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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