2
\$\begingroup\$

I've made a Pomodoro Timer application. All it does is countdown from a time in minutes given by the user to 0:00. It does this first for a session time, and then switches to a break timer and continuously loops like this until the application is paused or closed.

The time can be adjusted by clicking on a '+' or '-' beside the time.

In doing this project I wanted to learn how i could better organize my code as far as separation of concerns goes. Therefore I'm primarily looking for feedback as to how the the pieces of code could be better organized. I split it up between a model for the data and functions that change the data and a view to draw the application and set up event listeners.

Here is the Javascript :

'use strict';

$(function() {

  var model = (function() {

  var data = {
    minutes : null,
    temp : null,
    tempBreak : null,
    seconds : null,
    timer : {session : true, break : false},
    active : null // check to see if a timer interval is already set.
  };

  var timer;

  function changeTime() {

    data.seconds = (data.seconds > 0) ? data.seconds - 1 : 59;
    data.minutes = (data.minutes > 0 && data.seconds === 59) ? data.minutes - 1 : data.minutes;

    if (data.seconds === 0 && data.minutes === 0) {
      changeTimer();
    }

    view.render();
  }

  function changeTimer() {
    data.timer['session'] = !data.timer['session'];
    data.timer['break'] = !data.timer['break'];
    model.resetTimer();
  }

  return {
    init : function() {
      data.minutes = 25;
      data.temp = data.minutes;
      data.tempBreak = 5;
      data.seconds = 0;
      data.timer['session'] = true;
      data.timer['break'] = false;
      data.active = false;
    },
    incrementMinutes : function() {
      if (!data.active) {
         if (data.timer['session']) {
           data.temp++;
           data.minutes = data.temp;
         } else if (data.timer['break']) {
           data.tempBreak++;
           data.minutes = data.tempBreak;
        }

        model.resetTimer();
      }

      view.render();
    },
    decrementMinutes : function() {
      if (!data.active) {
        if (data.timer['session']) {
          data.temp = (data.temp > 0) ? data.temp - 1 : 0;
          data.minutes = data.temp;
        } else if (data.timer['break']) {
          data.tempBreak = (data.tempBreak > 0) ? data.tempBreak - 1 : 0;
          data.minutes = data.tempBreak;
        }

        model.resetTimer(); 
      }

      view.render();
    },
    getMinutes : function() {
      return data.minutes;
    },
    getSeconds : function() {
      return data.seconds;
    },
    switchToSession : function() {
      if (!data.active) {
        data.timer['session'] = true;
        data.timer['break'] = false;
        model.resetTimer();
        view.render(); 
      }
    },
    switchToBreak : function() {
      if (!data.active) {
        data.timer['session'] = false;
        data.timer['break'] = true;
        model.resetTimer();
        view.render(); 
      }
    },
    getTimerType : function() {
      return (data.timer['session']) ? 'Session' : 'Break';
    },
    startTimer : function() {
      data.active = !data.active;
      if (data.active){
          timer = setInterval(function() {
          changeTime();
        }, 1000); 
      } else {
        model.pauseTimer();
      }
    },
    pauseTimer : function() {
      data.active = false;
      clearInterval(timer);
    },
    resetTimer : function() {
      data.minutes = (data.timer['session']) ? data.temp : data.tempBreak;
      data.seconds = 0;
      view.render();
    }
  };
})();

var view = (function() {
  // Cache the DOM
  var sessionCheck = document.getElementById('session');
  var breakCheck = document.getElementById('break');

  var timerHeading = document.getElementById('timerType');

  var up = document.getElementById('up');
  var down = document.getElementById('down');

  var minutes = document.getElementById('minutes');
  var seconds = document.getElementById('seconds');

  var start = document.getElementById('start');
  var pause = document.getElementById('pause');
  var reset = document.getElementById('reset');

  // Bind events
  sessionCheck.addEventListener('mouseup', model.switchToSession);
  breakCheck.addEventListener('mouseup', model.switchToBreak);
  up.addEventListener('mouseup', model.incrementMinutes);
  down.addEventListener('mouseup', model.decrementMinutes);
  start.addEventListener('mouseup', model.startTimer);
  pause.addEventListener('mouseup', model.pauseTimer);
  reset.addEventListener('mouseup', model.resetTimer);

  var render = function() {
    minutes.innerHTML = model.getMinutes();
    seconds.innerHTML = (model.getSeconds() > 9) ? model.getSeconds() : '0' + model.getSeconds();
    timerHeading.innerHTML = model.getTimerType();
  }

  return {
    render : render
  };


})();

model.init();
view.render();

});

Here is the Pen: http://codepen.io/rfdeveloper/pen/vNXJLQ?editors=001

\$\endgroup\$
1
\$\begingroup\$

The internal representation of time can be different from how it is displayed, and is often a good idea to separate the too. For example, here you count down time in two steps:

  1. decrement seconds by 1, if negative then set to 59
  2. decrement minutes by 1 if seconds is 59

Consider using an internal representation as total seconds, let's say data.totalSeconds, initialize to 25 * 60 (for 25 minutes), and derive the display values from that.

That way, instead of this:

data.seconds = (data.seconds > 0) ? data.seconds - 1 : 59;
data.minutes = (data.minutes > 0 && data.seconds === 59) ? data.minutes - 1 : data.minutes;

if (data.seconds === 0 && data.minutes === 0) {
  changeTimer();
}

The counting down becomes a bit simpler (fewer conditions):

  data.totalSeconds -= 1;
  data.seconds = data.totalSeconds % 60;
  data.minutes = parseInt(data.totalSeconds / 60);

  if (data.totalSeconds === 0) {
    changeTimer();
  }
\$\endgroup\$
0
\$\begingroup\$

Timer modelling

With access to a system clock (via javascript's Date and Date() instance methods) :

  • a timer can be modelled as a single register start_time (plus a bunch of methods).
  • a pausable timer can be modelled as two registers cumulative_time and last_start_time (plus a bunch of methods).

Thus modelled, the basic timer paradigm is timerValue = cumulative_time + time_now - last_start_time. Only simple manipulation of timerValue value is necessary to provide count-up and count-down variants, and is a view consideration, not model.

We should see :

  • Javascript's Date.now() and Date() instance(s) somewhere in the code.

And we should not see :

  • setInterval(), reliance on which will lead to inaccuracy.
  • minutes and seconds anywhere other than in the view. Their presence in the model is indicative of not having fully separated out the view from the model.

Timer as a widget

Your timer is essentially a widget.

As such, it would be more easily invoked by passing in a single container element and having all the internal elements generated on view initialization.

Hence, most of the document.getElementById() statements would become document.createElement(...) statements (or their jQuery equivalents).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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