Problem
I have the following piece of code which hides an element when a specified date was reached. I would like to get some tips about do’s and don’ts.
Specifically, I’m interested in:
- improvements brought to this code
- avoid bad practices
And whatever you guys consider I should be careful about.
var Timer = (function(){
var $el = $('#element');
function count() {
setInterval(function() {
check();
}, 1000);
}
function hide() {
$el.hide();
}
function check() {
var currentDate = new Date();
var endingDate = new Date("July 25, 2016 11:06:00");
if (currentDate.getTime() >= endingDate.getTime()) {
hide();
}
}
return {
count: count
};
})();
Timer.count();
Solution
My notes:
- There is no need to create a function that calls
check
, you can just passcheck
- There is no need for function
hide
, you only call it once, and it only has 1 line of code, I would inline that function - Very minor, but there is no need to declare
endingDate
insidecheck
, I would declare it once outside of the function - You are using a revealing pattern, I would use a simple self executing pattern. On the whole if you expose 1 function that you will run only once, use a self executing function.
- Once the date is passed you will be hiding that element non-stop every second, that sounds like overkill
- I abhor anonymous functions, choose a meaningful name, and you can spare a line of comment 😉
My approach with these notes would be
(function HideElementWhenTimeComes (){
var $el = $('#element'),
endingDate = new Date("July 25, 2016 11:06:00"),
intervalID;
function check() {
var currentDate = new Date();
if (currentDate.getTime() >= endingDate.getTime()) {
$el.hide();
clearInterval(intervalID);
}
}
intervalID = setInterval(check , 1000 );
})();