Showing "open" or "closed" based on the current time I'm using this code to have a site show either &quot;Open&quot; or &quot;Closed&quot; depending on what the current time  is.
I'm a junior developer and I want to show this to my boss but one of the Senior Developers said to make it more robust/reusable before I do.


var currentDate = new Date();
var currentMonth = currentDate.getMonth();
var currentDay = currentDate.getDay();
var currentHour = currentDate.getHours();
var currentMinute = currentDate.getMinutes();

function openClose() {
  if(currentDay &gt;= 1 &amp;&amp; currentDay &lt;= 4) { 
    document.getElementById("friday").style.display = "none";
    if (currentHour &gt;= 8 &amp;&amp; currentHour &lt;= 21) { 
      document.getElementById("closed").style.display = "none";
    }
  else {
    document.getElementById("open").style.display = "none";
  }
}
if (currentDay === 5) { 
  document.getElementById("mon-thurs").style.display = "none";
  if (currentHour &gt;= 8 &amp;&amp; currentHour &lt;= 17.5) { 
    document.getElementById("closed").style.display = "none";
  }
  else {
    document.getElementById("open").style.display = "none";
  }
}
if (currentDay === 0 || currentDay === 6) { 
  document.getElementById("open").style.display = "none";
  document.getElementById("friday").style.display = "none";
}
};

setInterval(openClose(), 5000);
.layout-header .header-one #open,
.layout-header .header-one #closed,
.layout-header .header-one #friday,
.layout-header .header-one #mon-thurs {
   display: inline-block;
   color: #000;
}
&lt;div id="open-display"&gt;
 &lt;div id="open"&gt;Open&lt;/div&gt;
 &lt;div id="closed"&gt;Closed&lt;/div&gt;
 &lt;div id="mon-thurs"&gt;Sales: 8am - 8pm&lt;/div&gt;
 &lt;div id="friday"&gt;Sales: 8am - 5:30pm&lt;/div&gt;
&lt;/div&gt;
The first bug is that you are misusing setInterval().  You should be passing a function, not the void result of the function, when calling setInterval().

Even if you had used setInterval() correctly, you still have a second bug, which is that currentDate is set just once when the code loads.  If you call openClose() every five seconds, you would get the same result every time!

Third, the JavaScript sets .style.display = 'none', but never resets elements to be visible.  Therefore, even if the first two issues were corrected, your page would appear emptier and emptier through the course of the week.

The code should degrade gracefully in the absence of JavaScript.

Display decisions should be written in CSS.  Ideally, the JavaScript should refrain from making decisions like .style.display = 'none', and should restrict itself to doing the minimum necessary (i.e. setting classes) to let the CSS work.

There is a giant maintenance headache: the hours are encoded both in the HTML and the JavaScript.  It is likely that someone in the future would edit one but forget to edit the other accordingly.  Furthermore, the JavaScript representation is both cryptic (what does 17.5 mean?) and cumbersome (all those if conditions!).  I recommend changing the JavaScript code so that it interprets the opening hours from the HTML.  The easiest way would be to store the hours redundantly as machine-readable attributes in the HTML.



function isOpen(timeRangeEl, date) {
    var day = '' + date.getDay();
    var hhmm = ('0' + date.getHours()).slice(-2) + ':' + ('0' + date.getMinutes()).slice(-2);

    var days = timeRangeEl.getAttribute('data-days');
    var openTime = timeRangeEl.getAttribute('data-open');
    var closeTime = timeRangeEl.getAttribute('data-close');
    return days.indexOf(day) &gt;= 0 &amp;&amp; openTime &lt;= hhmm &amp;&amp; hhmm &lt; closeTime;
}

function openClose() {
    var date = new Date();
    var display = document.getElementById('open-display');
    var els = display.getElementsByClassName('timerange');
    var anyActive = false;
    for (var i = 0; i &lt; els.length; i++) {
        if (isOpen(els[i], date)) {
            anyActive = true;
            els[i].className = els[i].className.replace(/ *inactive\b/g, '');
        } else if (els[i].className.indexOf('inactive') &lt; 0) {
            els[i].className += ' inactive';
        }
    }
    if (anyActive) {
        display.className = 'open';
    } else {
        display.className = 'closed';
    }
}

setInterval(openClose, 5000);
openClose();
#open-display.open &gt; .timerange.inactive,
#open-display.open .days {
     display: none;
}
#open-display.open &gt; .openclosed::before {
    content: 'Open';
}
#open-display.closed &gt; .openclosed::before {
    content: 'Closed';
}
&lt;div id="open-display"&gt;
 &lt;div class="openclosed"&gt;&lt;/div&gt;
 &lt;div class="timerange" data-days="1,2,3,4" data-open="08:00" data-close="20:00"&gt;Sales: &lt;span class="days"&gt;Mon - Thu&lt;/span&gt; 8am - 8pm&lt;/div&gt;
 &lt;div class="timerange" data-days="5" data-open="08:00" data-close="17:30"&gt;Sales: &lt;span class="days"&gt;Fri&lt;/span&gt; 8am - 5:30pm&lt;/div&gt;
&lt;/div&gt;