Revealing store sales based on time

Question

I am delving into trying to write some JavaScript code. I understand the basics, (the very basics...) enough that I have been able to write the following and it works.

However, my knowledge doesn't stretch enough to be able to work out if this was the "best" way to write what I am trying to achieve. I don't want to just settle on it being "done" because it works, if there is a way to improve it and in turn, improve my understanding, I'd like to learn from people on best practices.

Currently, the code does the following:

If a user comes to the page before Sales Week, a message will display to the user telling them the days/hours/minutes until SalesWeek

(Please note that you won't be able to actually see the countdown at present on the code provided as it's a different piece of JS which is why it's in a funny span but it was written by a predecessor and I know it works.)

If a user comes to the page during sales week it will show which daily deal is open (and how long there is until it expires, using the same countdown timer as above which sadly is not included in this code but does work) and which deals have expired with a content div below for the actual sale items.

If the user comes to the page AFTER sales week they get a message to say that all the sales are over.

As a caveat, I know that using the client date is not ideal, and that server time should be used. Also I think this could have all been created in the back-end (where the deals couldn't be found out by someone looking at the source) but again, this isn't realistic for right now.

I'd really appreciate any guidance on my javascript itself, in the way I have formed it, and if there are better ways to achieve the same result.

Here is a demo on JSFiddle.

var date=new Date();
var year=date.getFullYear();
var month=date.getMonth();
var day=date.getDate();
var hours=date.getHours();
var minutes=date.getMinutes();




function SetDivContent() {


    if ((year == 2015 && month < 10) || (year == 2015 && month == 10 && day < 23) || (year == 2015 && month == 10 && day == 23 && hours < 01)) {  
        console.log("We are PRE sale week");
        document.getElementById("bem-sale-week-tocome").className = "show";
    }
    else if (year == 2015 && month == 10 && day >= 28) {
        console.log("sales week has expired");
        document.getElementById("bem-sale-week-ended").className = "show";
        document.getElementById("day-one").className = "bem-sale-week--item-expired";
        document.getElementById("day-two").className = "bem-sale-week--item-expired";
        document.getElementById("day-three").className = "bem-sale-week--item-expired";
        document.getElementById("day-four").className = "bem-sale-week--item-expired";
        document.getElementById("day-five").className = "bem-sale-week--item-expired";
    }
    else {
        console.log("We are IN Sales Week. sales Week is active");
        document.getElementById("bem-sale-week-currentdeal").className = "show";
        if ( (day=="23" && hours>="01") || (day=="24" && hours<="00") ) {
            document.getElementById("day-one").className = "bem-sale-week--item-open";
            document.getElementById("day-one-list").className += " open";
        }
        else if ( (day=="24" && hours>="01") || (day=="25" && hours<="00") ){
            document.getElementById("day-one").className = "bem-sale-week--item-expired";
            document.getElementById("day-two").className = "bem-sale-week--item-open";
            document.getElementById("day-two-list").className += " open";
        }
        else if ( (day=="25" && hours>="01") || (day=="26" && hours<="00") ){
            document.getElementById("day-one").className = "bem-sale-week--item-expired";
            document.getElementById("day-two").className = "bem-sale-week--item-expired";
            document.getElementById("day-three").className = "bem-sale-week--item-open";
            document.getElementById("day-three-list").className += " open";
        }
        else if( (day=="26" && hours>="01") || (day=="27" && hours<="00") ){
            document.getElementById("day-one").className = "bem-sale-week--item-expired";
            document.getElementById("day-two").className = "bem-sale-week--item-expired";
            document.getElementById("day-three").className = "bem-sale-week--item-expired";
            document.getElementById("day-four").className = "bem-sale-week--item-open";
            document.getElementById("day-four-list").className += " open";
        }
        else if( (day=="27" && hours>="01") || (day=="28" && hours<="00") ){
            document.getElementById("day-one").className = "bem-sale-week--item-expired";
            document.getElementById("day-two").className = "bem-sale-week--item-expired";
            document.getElementById("day-three").className = "bem-sale-week--item-expired";
            document.getElementById("day-four").className = "bem-sale-week--item-expired";
            document.getElementById("day-five").className = "bem-sale-week--item-open";
            document.getElementById("day-five-list").className += " open";
        }

    }

}
window.onload = SetDivContent;

Answer 1

Because of the way that if-else statements work, you can eliminate redundancies.

Here's the code cut up some to focus on just the if-else statements:

if ((year == 2015 && month < 10) || (year == 2015 && month == 10 && day < 23) || (year == 2015 && month == 10 && day == 23 && hours < 01)) { 
// do things
}
else if (year == 2015 && month == 10 && day >= 28) { 
}
else {
    if ( (day=="23" && hours>="01") || (day=="24" && hours<="00") ) {

    }
    else if ( (day=="24" && hours>="01") || (day=="25" && hours<="00") ){

    }
    else if ( (day=="25" && hours>="01") || (day=="26" && hours<="00") ){
    }
    else if( (day=="26" && hours>="01") || (day=="27" && hours<="00") ){
    }
    else if( (day=="27" && hours>="01") || (day=="28" && hours<="00") ){
    }
}

The first statement first checks the leftmost set of parentheses, then the next one, etc. But these all have one thing in common: year == 2015. We can extract that so it doesn't check it so much.

    if (year == 2015 && (month < 10 //indentation can help readability
                        || (month == 10 && day < 23) //or not! it's just preference
                        || (month == 10 && day == 23 && hours < 01))
    ) { 
    // do things
    }
    // altering this statement too
    else if (year == 2015 && (month == 10 && day >= 28)) { 
    }

Now, this portion will evaluate the same way, but it's drier, will only check year a maximum of twice (instead of five before) and easier to edit later.

---

You can also extract the strings "bem-sale-week--item-expired" & "bem-sale-week--item-open" into vars, up with the rest of your variable declarations:

var date = new Date();
var year = date.getFullYear(),
    month = date.getMonth(),
    day = date.getDate(),
    hours = date.getHours(),
    minutes = date.getMinutes(),
    ITEM_EXPIRED = "bem-sale-week--item-expired",
    ITEM_OPEN = "bem-sale-week--item-open";

I added whitespace around the equals sign. I also changed the declarations to use the comma format and indentation (except for var date, which has to initialize before year, month, et al can call its methods). This isn't super important here, but it's good to know. Try to use the comma format particularly when you are repeating it a lot, like in a loop, as each call to "var" is a little work.

---

And then another thing that's good is comments!

// if we're in october 2015 before the 23rd: if (year == 2015 && (month < 10 || (month == 10 && day < 23) || (month == 10 && day == 23 && hours < 01)) ) { // ... } // else if we're in october 2015 after the 27th: else if (year == 2015 && (month == 10 && day >= 28)) { // ... } // else: we want to specify actions for each of the sale days (23rd thru 27th) else { // ...

Hope this helps!

(P.S.: I don't know what behavior you want for 2016 and later, but right now it might be unaccounted for!)

Answer 2

You may use loops to avoid duplication, like:

        document.getElementById("day-one").className = "bem-sale-week--item-expired";
        document.getElementById("day-two").className = "bem-sale-week--item-expired";
        document.getElementById("day-three").className = "bem-sale-week--item-expired";
        document.getElementById("day-four").className = "bem-sale-week--item-open";

to:

for day in ("day-one", "day-two" ...) {
    document.getElementById(day).className = "bem-sale-week--item-expired";

(Pseudo-code, actual for-each in java-script is left for you to discover)

---

And "bem-sale-week--item-open"; should be a constants as you use it everywhere, like:

const EXPIRED = "bem-sale-week--item-expired"

The same for:

const OPEN = "bem-sale-week--item-expired"

Answer 3

Use UTC Time to avoid timezone issues

People in different timezones will see different values for the current date and hour in JavaScript even when viewing the web page simultaneously. This is determined by the user's current timezone as specified on their system.

Generally, when counting down to a specific date, you want the countdown to be the same for everyone, no matter their system's timezone. You can use UTC times to retrieve, set, and compare time values against Universal Coordinated Time (which roughly corresponds to the old standard Greenwich Mean Time (GMT) but ignores daylight savings time).

If you have a date object, you can call myDateObject.toUTCString() to see what time it corresponds to in UTC. You can also call all the expected getter and setter functions to retrieve or update specific components of the object, such as .getUTCFullYear(), .getUTCMonth(), etc.

I would create a startDate variable using var startDate = new Date() and then set its date components using the UTC setters. You'll then be able to compare the current date against that variable and JavaScript will be smart enough to take timezone differences into account.

var currDT = new Date();
document.getElementById("hour").value = currDT.getHours();
document.getElementById("minute").value = currDT.getMinutes();
document.getElementById("second").value = currDT.getSeconds();
document.getElementById("date").value = currDT.getDate();
document.getElementById("month").value = currDT.getMonth();
document.getElementById("year").value = currDT.getFullYear();

document.getElementById("submit").addEventListener("click",function(){
  var hour = document.getElementById("hour").value;
  var minute = document.getElementById("minute").value;
  var second = document.getElementById("second").value;
  var date = document.getElementById("date").value;
  var month = document.getElementById("month").value;
  var year = document.getElementById("year").value;
  
  var utcDateTime = new Date();
  
  utcDateTime.setUTCFullYear(year);
  utcDateTime.setUTCMonth(month);
  utcDateTime.setUTCDate(date);
  utcDateTime.setUTCHours(hour);
  utcDateTime.setUTCMinutes(minute);
  utcDateTime.setUTCSeconds(second);
  utcDateTime.setUTCMilliseconds(0);
  
  document.getElementById("output").innerHTML = utcDateTime.toUTCString() + " is equivalent to " +utcDateTime.toString();
  
});

.table {display:table;}
.row{display:table-row;}
.cell{display:table-cell;font-family:courier;}

What UTC date would you like to see represented in local time?<br/>
<div class="table">
<div class="row">
  <div class="cell">
D:<input id="date"/><br/>M:<input id="month"/><br/>Y:<input id="year"/>
    </div>
  <div class="cell">H:<input id="hour"/> <br/>M:<input id="minute"/> <br/>S:<input id="second"/></div>
  
  </div>
  </div>
<input type="button" id="submit" value="Convert UTC to Local Time!"/>
<div id="output"></div>

This brings me to my next point...

Date objects can be compared directly

Instead of comparing hours to hours, years to years, months to months, and so on, you can compare two date objects directly to see which one comes earlier or later. You can even subtract one date object from the other to find the time (in milliseconds) between the two dates.

The code below will populate the dateDifference variable with a number representing the number of days since the sale started (assuming startDate is the sale's start date and currentDate is the current date).

var dateDifference = (currentDate - startDate) / 86400000 >>> 0;

We divide it by 86,400,000 just to convert milliseconds to days. The ... >>> 0 bitwise operation just truncates off any decimals for us, converting the number to an integer.

If the sale hasn't started yet, dateDifference will be a negative number; otherwise it well tell us which day of the sale we're currently on (using array index notation, so 0 for the first day, 1 for the second day, and so on).

Hopefully that's got you thinking about neat ways to programmatically determine which elements to show or hide.

Try not to repeat yourself, especially when it comes to strings

It's not much fun to have to go through your code and update a bunch of strings when something changes nine months down the road. You can make it easier on yourself by storing repeated strings of text into variables.

var expiredClass = "bem-sale-week--item-expired";
var openClass = "bem-sale-week--item-open";

Since these are constants, a common standard is to name the variables with all capital letters; you can do that if you find it helpful, but the important thing is to be internally consistent.

I would take this concept (of capturing reusable strings in variables) a step farther for the element IDs of your daily deals, and throw them into an array of objects.

var dayElements = [
{id: "day-one",list: "day-one-list"}, 
{id: "day-two",list: "day-two-list"}, 
{id: "day-three",list: "day-three-list"}, 
{id: "day-four",list: "day-four-list"},
{id: "day-five",list: "day-five-list"}];

This gives us a few nice advantages. Remember how earlier I said you could store the current day of the sale in a dateDifference variable? Well, you could use that to access the corresponding day and list elements without having to create a bunch of else if statements.

Your final SetDivContent method might end up looking like this:

function SetDivContent() {
    var currentDate = new Date();
    var dateDifference = (currentDate - startDate) / 86400000 >> 0;
    if (dateDifference < 0) { // sale hasn't started yet
        document.getElementById("bem-sale-week-tocome").className = "show";
    } else if (dateDifference >= dayElements.length) { // sale is over
        document.getElementById("bem-sale-week-ended").className = "show";
    } else {
        document.getElementById(dayElements[dateDifference].list).className += "open";
    }
    for (var i = 0; i < dayElements.length; i++) {
        if (dateDifference === i) {
            document.getElementById(dayElements[i].id).className = openClass;
        } else if (dateDifference > i) {
            document.getElementById(dayElements[i].id).className = expiredClass;
        }
    }
}

This way your script is almost good to go if you decide to have another sale that spans a different number of days. You'd just need to update the dayElements array to reflect the elements on the page (and update the startDate of course).