Pomodoro Clock JS Modularisation

Question

I am beginner on Javascript and i made a pomodoro clock for freecodecamp project with vanilla Javascript.

I was read few articles about modular and functional javascript and i want to use these techniques. But when i start writing code, I find it difficult to use these techniques. I will appreciated if anyone review my code and refactor it with modular patterns. Then I can compare the two codes, and that helps me a lot. And if there are bad implementations, please tell me my mistakes.

var addSession = document.querySelector('.add_session');
var decSession = document.querySelector('.dec_session');
var addBreak = document.querySelector('.add_break');
var decBreak = document.querySelector('.dec_break');
var sessionInput = parseInt(document.querySelector('.session_min').innerHTML);
document.querySelector('.session_min').innerHTML = sessionInput;
document.getElementById('counter').innerHTML = sessionInput;
var breakInput = document.querySelector('.break_min').innerHTML;
	breakInput = parseInt(breakInput);
document.querySelector('.break_min').innerHTML = breakInput;
var counterVal = document.getElementById('counter').innerHTML;
	counterVal = parseInt(counterVal);
	counterVal = sessionInput;
	counterVal = counterVal * 60;
document.getElementById('counter').innerHTML = counterVal;
document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
var startBtn = document.querySelector('.start');
var resetBtn = document.querySelector('.reset');
var breakBtn = document.querySelector('.break');
addSession.onclick = function(e) {
	sessionInput += 5;
	document.querySelector('.session_min').innerHTML = sessionInput;
	counterVal = sessionInput * 60;
	document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
};
decSession.onclick = function(e) {
	if (sessionInput > 5) {
		sessionInput -= 5;
		document.querySelector('.session_min').innerHTML = sessionInput;
		counterVal = sessionInput * 60;
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
	}
};
addBreak.onclick = function(e) {
	breakInput += 1;
	document.querySelector('.break_min').innerHTML = breakInput;
	counterVal = breakInput * 60;
	document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
};
decBreak.onclick = function(e) {
	if (breakInput > 1) {
		breakInput -= 1;
		document.querySelector('.break_min').innerHTML = breakInput;
		counterVal = breakInput * 60;
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
	}
};
function timer() {
 	counterVal -= 1;
 	if (counterVal % 60 >= 10) {
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + counterVal % 60;
 	} else {
 		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 	}
 	if (counterVal === 0) {
 		clearInterval(counter);
 		startBtn.innerHTML = 'Start';
 	}
}
startBtn.addEventListener('click', function() {
	
	if (startBtn.innerHTML === 'Start' && counterVal !== 0) {
		breakBtn.disabled = true;
		startBtn.innerHTML = 'Pause';
		counter = setInterval(timer, 1000);
	} else if (startBtn.innerHTML === 'Pause' && counterVal > 0) {
		breakBtn.disabled = false;
		startBtn.innerHTML = 'Start';
		clearInterval(counter);
	}
}, false);
resetBtn.addEventListener('click', function() {
	if (startBtn.innerHTML !== 'Pause') {
		counterVal = sessionInput;
		counterVal = counterVal * 60;
		document.getElementById('counter').innerHTML = counterVal;
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
	}
	if (breakBtn.innerHTML === "Work") {
		breakBtn.innerHTML = "Break";
	}
}, false);
breakBtn.addEventListener('click', function(){
	if (breakBtn.innerHTML === "Break") {
		addBreak.disabled = false;
		decBreak.disabled = false;
		addSession.disabled = true;
		decSession.disabled = true;
		breakBtn.innerHTML = "Work";
		counterVal = breakInput;
		counterVal = counterVal * 60;
		document.getElementById('counter').innerHTML = counterVal;
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
	} else if (breakBtn.innerHTML === "Work") {
		addBreak.disabled = true;
		decBreak.disabled = true;
		addSession.disabled = false;
		decSession.disabled = false;
		breakBtn.innerHTML = "Break";
		counterVal = sessionInput * 60;
		document.getElementById('counter').innerHTML = counterVal;
		document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
	}
	
}, false);

<div class="clock_wrapper">
	<h1>Pomodoro Clock</h1>
	<div class="clock">
		<h2 id="counter"></h2>
		<div class="buttons_wrapper">
			<button class="start">Start</button>
			<button class="reset">Reset</button>
			<button class="break">Break</button>
		</div>
		<hr/>
	</div>
	<div class="session_wrap">
		<h3>Session Time</h3>
		<div class="session_time">
			<button class="dec_session bt">-</button>
			<div class="session_min">25</div>
			<button class="add_session bt">+</button>
		</div>
		
	</div>
	<div class="break_wrap">
		<h3>Break Time</h3>
		<div class="break_time">
			<button disabled class="dec_break bt">-</button>
			<div class="break_min">5</div>
			<button disabled class="add_break bt">+</button>
		</div>
	</div>
	
</div>

Answer 1

First of all delete marker from HTMl ;-).

First bad thing in your code is a global scope polution. Currently every var and functions 'lives' in global. Check this: https://stackoverflow.com/questions/10525582/why-are-global-variables-considered-bad-practice

var addSession = document.querySelector('.add_session');
var decSession = document.querySelector('.dec_session');
var addBreak = document.querySelector('.add_break');
var decBreak = document.querySelector('.dec_break');
var sessionInput = parseInt(document.querySelector('.session_min').innerHTML);

Bad idea to get state values from html. It should be oposite to this aproach. For example var sessionInput = 25; And then sett it in html.

document.querySelector('.session_min').innerHTML = sessionInput;
document.getElementById('counter').innerHTML = sessionInput;

Be more consistent. You can use document.querySelector('#counter').;

var breakInput = document.querySelector('.break_min').innerHTML;
 breakInput = parseInt(breakInput);
document.querySelector('.break_min').innerHTML = breakInput;

So you got some states in app to meanage. The good idea is to put them together into one object called state. It will make debuging a piece of cake. For example:

var clockState = {
 breakTime = 0,
 sessionTime = 0,
 counterVal = 0
} 

Next you can use them like this:

someFunction( clockState.breakTime );

This below is quite messy:

var counterVal = document.getElementById('counter').innerHTML;
 counterVal = parseInt(counterVal);
 counterVal = sessionInput;
 counterVal = counterVal * 60;
document.getElementById('counter').innerHTML = counterVal;
document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;

Below better way to solve task like this:

counter = document.querySelector('#counter');
clockState.counterVal = 25 * 60;
counter.innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;

All variables should be declared at the begining. It makes easier to find them later ;-).

var startBtn = document.querySelector('.start');
var resetBtn = document.querySelector('.reset');
var breakBtn = document.querySelector('.break');

This is old way to meanage events in JS.

addSession.onclick = function(e) {
 sessionInput += 5;
 document.querySelector('.session_min').innerHTML = sessionInput;
 counterVal = sessionInput * 60;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
};

Today people are using addEventListener:

function modifySession(){/* Doing the work*/}
addSession.addEventListener('click',modifySession)

//

decSession.onclick = function(e) {
 if (sessionInput > 5) {
 sessionInput -= 5;
 document.querySelector('.session_min').innerHTML = sessionInput;
 counterVal = sessionInput * 60;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 }
};
addBreak.onclick = function(e) {
 breakInput += 1;
 document.querySelector('.break_min').innerHTML = breakInput;
 counterVal = breakInput * 60;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
};
decBreak.onclick = function(e) {
 if (breakInput > 1) {
 breakInput -= 1;
 document.querySelector('.break_min').innerHTML = breakInput;
 counterVal = breakInput * 60;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 }
};
function timer() {
 counterVal -= 1;
 if (counterVal % 60 >= 10) {
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + counterVal % 60;
 } else {
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 }
 if (counterVal === 0) {
 clearInterval(counter);
 startBtn.innerHTML = 'Start';
 }
}
startBtn.addEventListener('click', function() {
 if (startBtn.innerHTML === 'Start' && counterVal !== 0) {
 breakBtn.disabled = true;
 startBtn.innerHTML = 'Pause';
 counter = setInterval(timer, 1000);
 } else if (startBtn.innerHTML === 'Pause' && counterVal > 0) {
 breakBtn.disabled = false;
 startBtn.innerHTML = 'Start';
 clearInterval(counter);
 }
}, false);
resetBtn.addEventListener('click', function() {
 if (startBtn.innerHTML !== 'Pause') {
 counterVal = sessionInput;
 counterVal = counterVal * 60;
 document.getElementById('counter').innerHTML = counterVal;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 }
 if (breakBtn.innerHTML === "Work") {
 breakBtn.innerHTML = "Break";
 }
}, false);
breakBtn.addEventListener('click', function(){
 if (breakBtn.innerHTML === "Break") {
 addBreak.disabled = false;
 decBreak.disabled = false;
 addSession.disabled = true;
 decSession.disabled = true;
 breakBtn.innerHTML = "Work";
 counterVal = breakInput;
 counterVal = counterVal * 60;
 document.getElementById('counter').innerHTML = counterVal;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 } else if (breakBtn.innerHTML === "Work") {
 addBreak.disabled = true;
 decBreak.disabled = true;
 addSession.disabled = false;
 decSession.disabled = false;
 breakBtn.innerHTML = "Break";
 counterVal = sessionInput * 60;
 document.getElementById('counter').innerHTML = counterVal;
 document.getElementById('counter').innerHTML = Math.floor(counterVal / 60) + ":" + "0" + counterVal % 60;
 }
}, false);

Currently your application is about 130 lines long. Try to make it 65. And learn the DRY principle: https://en.wikipedia.org/wiki/Don%27t_repeat_yourself.

If you are looking for challenges then try to write simple calculator.

• erenesto: thank you. I will try to apply what you say. By the way i made a simple calculator before pomodoro clock