Pomodoro timer module

Question

I have been learning javascript for some time and have just finished my first piece of code that not only actually does something, but is also longer than a few lines of code. It most likely is pretty terrible though (but works!). I would appreciate any help, insight or/and pointers.

General idea was to create a module, that I can import, will generate HTML and do the timer things, and expose simple API while keeping most of the guts private (trying to wrap my head around general idea of modules and some popular patterns).

var timer_module = function (wrkTime, brkTime, parentEl) {
var locRef = {
 workTime: wrkTime ? wrkTime : 25,
 breakTime: brkTime ? brkTime : 10,
 seconds: 0,
 isPlaying: false,
 isBreak: false
 },
 seconds = locRef.seconds,
 minutes = locRef.workTime,
 pubAcc = {},
 secondsHandle, sdiv, mdiv, resetButton, startPauseButton;
(function render_DOM_elements(){
 var array_of_elements = [
 {tag: 'section', id: 'tm_container', parent: parentEl, initialContent: ''},
 {tag: 'div', id: 'tm_minutes', parent: 'tm_container', initialContent: ''},
 {tag: 'div', id: 'tm_seconds', parent: 'tm_container', initialContent: ''},
 {tag: 'div', id: 'tm_bcontainer', parent: 'tm_container', initialContent: ''},
 {tag: 'button', id: 'tm_playStopButton', parent: 'tm_bcontainer',
 initialContent: '<i class="fa fa-play-circle-o" aria-hidden="true"></i>'},
 {tag: 'button', id: 'tm_resetButton', parent: 'tm_bcontainer',
 initialContent: '<i class="fa fa-stop-circle-o" aria-hidden="true"></i>'}
 ];
 function create_element(elementObject){
 var el = document.createElement(elementObject.tag);
 var parent = document.getElementById(elementObject.parent);
 el.setAttribute('id', elementObject.id);
 el.innerHTML = elementObject.initialContent;
 if(!parent){
 document.body.appendChild(el)
 } else {
 parent.appendChild(el)
 }
 }
 array_of_elements.forEach(create_element);
 sdiv = document.getElementById('tm_seconds');
 mdiv = document.getElementById('tm_minutes');
 startPauseButton = document.getElementById('tm_playStopButton');
 resetButton = document.getElementById('tm_resetButton');
})();
//update the dom/display and format(zero-pad) the time
function updateTime() {
 sdiv.innerHTML = seconds < 10 ? '0' + seconds.toString() : seconds;
 mdiv.innerHTML = minutes < 10 ? '0' + minutes.toString() : minutes;
}
//change the guts of buttons, start/pause
function play_reset_change() {
 var play_icon = '<i class="fa fa-play-circle-o" aria-hidden="true"></i>',
 pause_icon = '<i class="fa fa-pause-circle-o" aria-hidden="true"></i>';
 startPauseButton.innerHTML = locRef.isPlaying ? pause_icon : play_icon
}
//steInterval guts
function minusSec() {
 if (minutes === 0 && seconds === 0) {
 if (!locRef.isBreak) {
 locRef.isBreak = true;
 minutes = locRef.breakTime;
 seconds = 0;
 updateTime();
 } else {
 locRef.isBreak = true;
 minutes = locRef.workTime;
 seconds = 0;
 updateTime();
 }
 } else if (minutes !== 0 && seconds === 0) {
 seconds = 59;
 minutes--;
 updateTime();
 } else {
 seconds--;
 updateTime();
 }
}
//initial time display
updateTime();
pubAcc.start = function () {
 locRef.isPlaying = true;
 //updateTime();
 play_reset_change();
 secondsHandle = setInterval(minusSec, 1000);
};
pubAcc.pause = function () {
 locRef.isPlaying = false;
 play_reset_change();
 clearInterval(secondsHandle);
};
pubAcc.reset = function () {
 locRef.isPlaying = false;
 locRef.isBreak = false;
 clearInterval(secondsHandle);
 minutes = locRef.workTime;
 seconds = 0;
 play_reset_change();
 updateTime();
};
startPauseButton.addEventListener('click', function () {
 if (!locRef.isPlaying) {
 pubAcc.start();
 } else {
 pubAcc.pause();
 }
});
resetButton.addEventListener('click', function () {
 pubAcc.reset();
});
return pubAcc;
};
var timer1 = timer_module(15,7,'i_want_pomodoro_here');

• 200_success: Welcome to Code Review! Consider making a live demo: press Ctrl-M in the question editor.

Answer 1

I've put the your code into a Code Snippet below, as well as a jsFiddle so anyone can easily see how it runs.

var timer_module = function(wrkTime, brkTime, parentEl) {
 var locRef = {
 workTime: wrkTime ? wrkTime : 25,
 breakTime: brkTime ? brkTime : 10,
 seconds: 0,
 isPlaying: false,
 isBreak: false
 },
 seconds = locRef.seconds,
 minutes = locRef.workTime,
 pubAcc = {},
 secondsHandle, sdiv, mdiv, resetButton, startPauseButton;
 (function render_DOM_elements() {
 var array_of_elements = [{
 tag: 'section',
 id: 'tm_container',
 parent: parentEl,
 initialContent: ''
 }, {
 tag: 'div',
 id: 'tm_minutes',
 parent: 'tm_container',
 initialContent: ''
 }, {
 tag: 'div',
 id: 'tm_seconds',
 parent: 'tm_container',
 initialContent: ''
 }, {
 tag: 'div',
 id: 'tm_bcontainer',
 parent: 'tm_container',
 initialContent: ''
 }, {
 tag: 'button',
 id: 'tm_playStopButton',
 parent: 'tm_bcontainer',
 initialContent: '<i class="fa fa-play-circle-o" aria-hidden="true"></i>'
 }, {
 tag: 'button',
 id: 'tm_resetButton',
 parent: 'tm_bcontainer',
 initialContent: '<i class="fa fa-stop-circle-o" aria-hidden="true"></i>'
 }];
 function create_element(elementObject) {
 var el = document.createElement(elementObject.tag);
 var parent = document.getElementById(elementObject.parent);
 el.setAttribute('id', elementObject.id);
 el.innerHTML = elementObject.initialContent;
 if (!parent) {
 document.body.appendChild(el)
 } else {
 parent.appendChild(el)
 }
 }
 array_of_elements.forEach(create_element);
 sdiv = document.getElementById('tm_seconds');
 mdiv = document.getElementById('tm_minutes');
 startPauseButton = document.getElementById('tm_playStopButton');
 resetButton = document.getElementById('tm_resetButton');
 })();
 //update the dom/display and format(zero-pad) the time
 function updateTime() {
 sdiv.innerHTML = seconds < 10 ? '0' + seconds.toString() : seconds;
 mdiv.innerHTML = minutes < 10 ? '0' + minutes.toString() : minutes;
 }
 //change the guts of buttons, start/pause
 function play_reset_change() {
 var play_icon = '<i class="fa fa-play-circle-o" aria-hidden="true"></i>',
 pause_icon = '<i class="fa fa-pause-circle-o" aria-hidden="true"></i>';
 startPauseButton.innerHTML = locRef.isPlaying ? pause_icon : play_icon
 }
 //steInterval guts
 function minusSec() {
 if (minutes === 0 && seconds === 0) {
 if (!locRef.isBreak) {
 locRef.isBreak = true;
 minutes = locRef.breakTime;
 seconds = 0;
 updateTime();
 } else {
 locRef.isBreak = true;
 minutes = locRef.workTime;
 seconds = 0;
 updateTime();
 }
 } else if (minutes !== 0 && seconds === 0) {
 seconds = 59;
 minutes--;
 updateTime();
 } else {
 seconds--;
 updateTime();
 }
 }
 //initial time display
 updateTime();
 pubAcc.start = function() {
 locRef.isPlaying = true;
 //updateTime();
 play_reset_change();
 secondsHandle = setInterval(minusSec, 1000);
 };
 pubAcc.pause = function() {
 locRef.isPlaying = false;
 play_reset_change();
 clearInterval(secondsHandle);
 };
 pubAcc.reset = function() {
 locRef.isPlaying = false;
 locRef.isBreak = false;
 clearInterval(secondsHandle);
 minutes = locRef.workTime;
 seconds = 0;
 play_reset_change();
 updateTime();
 };
 startPauseButton.addEventListener('click', function() {
 if (!locRef.isPlaying) {
 pubAcc.start();
 } else {
 pubAcc.pause();
 }
 });
 resetButton.addEventListener('click', function() {
 pubAcc.reset();
 });
 return pubAcc;
};
var timer1 = timer_module(15, 7, 'i_want_pomodoro_here');

<!DOCTYPE html>
<html>
<head>
 <meta name="description">
 <meta charset="utf-8">
 <meta name="viewport" content="width=device-width">
 <title>JS Bin</title>
 <link href="https://maxcdn.bootstrapcdn.com/font-awesome/4.7.0/css/font-awesome.min.css" rel="stylesheet" type="text/css" />
</head>
<body>
 <div id="i_want_pomodoro_here">
 </div>
</body>
</html>

My first piece of advice is to try to avoid overusing the comma notation for declaring variables:

var locRef = {
 workTime: wrkTime ? wrkTime : 25,
 breakTime: brkTime ? brkTime : 10,
 seconds: 0,
 isPlaying: false,
 isBreak: false
 },
 seconds = locRef.seconds,
 minutes = locRef.workTime,
 pubAcc = {},
 secondsHandle, sdiv, mdiv, resetButton, startPauseButton;

This is perfectly valid JavaScript. But it looks a bit confusing at first glance, since you go from one line per variable to multiple variables per line, all strung together with commas. There's no one correct way to do this, but in this case I would stick with one var per line.

var locRef = {
 workTime: wrkTime ? wrkTime : 25,
 breakTime: brkTime ? brkTime : 10,
 seconds: 0,
 isPlaying: false,
 isBreak: false
};
var seconds = locRef.seconds;
var minutes = locRef.workTime;
var pubAcc = {};
var secondsHandle, sdiv, mdiv, resetButton, startPauseButton;

It's a style issue, so feel free to ignore my advice if you feel that the above code doesn't look right to you.