1

I have two "stopwatches" in my code (and I may be adding more). This is the code I currently use below - and it works fine. But I'd really like to put the bulk of that code into a function so I'm not repeating the same code over and over.

When I tried doing it though, I could get it working - I think it was because I was passing stopwatchTimerId and stopwatch2TimerId into the function and it may have been passing by reference?

How can I reduce the amount of code repetition here?

var stopwatchTimerId = 0;
var stopwatch2TimerId = 0;

$('#stopwatch').click(function () {
    if ($(this).hasClass('active')) {
        $(this).removeClass('active');

        clearInterval(stopwatchTimerId);
    }
    else {
        $(this).addClass('active');

        stopwatchTimerId = setInterval(function () {
            var currentValue = parseInt($('#stopwatch-seconds').val()) || 0;
            $('#stopwatch-seconds').val(currentValue + 1).change();
        }, 1000);
    }
});

$('#stopwatch2').click(function () {
    if ($(this).hasClass('active')) {
        $(this).removeClass('active');

        clearInterval(stopwatch2TimerId);
    }
    else {
        $(this).addClass('active');

        stopwatch2TimerId = setInterval(function () {
            var currentValue = parseInt($('#stopwatch2-seconds').val()) || 0;
            $('#stopwatch2-seconds').val(currentValue + 1).change();
        }, 1000);
    }
});

As you can see, it's basically the same code in each except for stopwatchTimerId and $('#stopwatch-seconds') (and the same vars with 2 on it for the other one).

b85411
  • 9,420
  • 15
  • 65
  • 119

4 Answers4

4

This won't pollute global scope and also you don't need to do any if-else statements. Just add data-selector to your new elements :)

<input id="stopwatch" type="text" data-selector="#stopwatch-seconds"/>
<input id="stopwatch2" type"text" data-selector="#stopwatch2-seconds"/>

$('#stopwatch stopwatch2').click(function () {

    var $element = $(this),
        interval = $element.data('interval');
        selector = $element.data('selector');;
    if ($element.hasClass('active')) {
        $element.removeClass('active');

        if (interval) {
            clearInterval(interval);
        }
    }
    else {
        $element.addClass('active');

        $element.data('interval', setInterval(function () {


            var currentValue = parseInt($(selector).val()) || 0;
            $(selector).val(currentValue + 1).change();


        }, 1000));
    }
});
Vigneswaran Marimuthu
  • 2,452
  • 13
  • 16
  • That is seriously, SERIOUSLY classy! I love this solution! Thanks so much, what an elegant way to do it! I have one question - why do you call it `var $element` (with the dollar sign) instead of `var element`? – b85411 Apr 14 '15 at 04:48
  • Because it is a jquery wrapper :) http://stackoverflow.com/questions/1916584/jquery-variable-syntax – Vigneswaran Marimuthu Apr 14 '15 at 05:00
1
function stopwatch(id){
  $('#' + id).click(function () {
    if ($(this).hasClass('active')) {
        $(this).removeClass('active');

        clearInterval(window[id]);
    }
    else {
        $(this).addClass('active');

        window[id] = setInterval(function () {
        var currentValue = parseInt($('#' + id + '-seconds').val()) || 0;
        $('#' + id + '-seconds').val(currentValue + 1).change();
    }, 1000);
}
});
}
$(function(){
  stopwatch("stopwatch");
  stopwatch("stopwatch2");
});
AxelPAL
  • 1,047
  • 3
  • 12
  • 19
0

You could do something like this (code is not very nice, you can improve it):

var stopwatchTimerId;
$('#stopwatch').click(function () {
        doStopWatch(1);
    });

    $('#stopwatch2').click(function () {
        doStopWatch(2);
    });

    var doStopWatch = function(option){
        var stopWatch = option===1?$('#stopwatch'):$('#stopwatch2');
        if (stopWatch.hasClass('active')) {
            stopWatch.removeClass('active');

            clearInterval(stopwatchTimerId);
        }
        else {
            stopWatch.addClass('active');
            stopwatchTimerId = setInterval(function () {
                var currentValue = option===1?(parseInt($('#stopwatch-seconds').val()) || 0):(parseInt($('#stopwatch2-seconds').val()) || 0);
                if(option===1)
                    $('#stopwatch-seconds').val(currentValue + 1).change();
                else 
                    $('#stopwatch2-seconds').val(currentValue + 1).change();
            }, 1000);
        }
    }
user4341206
  • 647
  • 1
  • 7
  • 26
0

Try

var arr = $.map($("div[id^=stopwatch]"), function(el, index) {
  el.onclick = watch;
  return 0
});

function watch(e) {
  var id = this.id;
  var n = Number(id.split(/-/)[1]);
  if ($(this).hasClass("active")) {
    $(this).removeClass("active");
    clearInterval(arr[n]);
  } else {
    $(this).addClass("active");
    arr[n] = setInterval(function() {
      var currentValue = parseInt($("#" + id + "-seconds").val()) || 0;
      $("#" + id + "-seconds").val(currentValue + 1).change();
    }, 1000);
  }
};
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js">
</script>
<div id="stopwatch-0">stopwatch1</div>
<input type="text" id="stopwatch-0-seconds" />
<div id="stopwatch-1">stopwatch2</div>
<input type="text" id="stopwatch-1-seconds" />
guest271314
  • 1
  • 15
  • 104
  • 177