Change state depending on holidays, weekends or hour of day

Question

I'm writing a script that's changing the background color depending on hour of day, if it is a weekend or if it is a Swedish holiday. I have a functional script and as I want to improve my programming skills I want to see if there's anything I can do differently or more efficiently.

I wanted to work with separate functions to (hopefully) improve readability and make the script more adaptable by passing parameters. The script also runs a function on an interval to update the background color if the allowed time span has been passed. My concern is that this will affect performance as far as I know it will not be too CPU intensive.

	closedDates();

	function closedDates() {
		//Create new date object
		var d = new Date();
		var year = d.getFullYear();

		var closedDates = (calculateClosedDates(year));

		var closedDates = [
			new Date(year, 0, 1), 		// Fixed date: New Years day
			new Date(year, 0, 5), 		// Fixed date: Twelfth Night
			new Date(year, 0, 6), 		// Fixed date: Epiphany
			new Date(closedDates[0]),	// Thursday
			new Date(closedDates[1]), 	// Good Friday
			new Date(closedDates[2]), 	// Easter Day
			new Date(closedDates[3]), 	// Easter Monday
			new Date(year, 3, 30), 		// Fixed date: Walpurgis Night
			new Date(year, 4, 1),		// Fixed date: International Workers' Day
			new Date(closedDates[4]),	// Feast of the Ascension
			new Date(closedDates[5]),	// Pingstafton
			new Date(closedDates[6]),	// Pingstdagen
			new Date(year, 5, 6), 		// Fixed date: Swedish National Day
			new Date(closedDates[7]),	// Saint John's Eve
			new Date(closedDates[8]),	// Midsummer day
			new Date(closedDates[9]),	// All Saints' Eve
			new Date(closedDates[10]),	// All Saints' Day
			new Date(year, 11, 24),		// Fixed date: Christmas Eve
			new Date(year, 11, 25),		// Fixed date: Christmas Day
			new Date(year, 11, 26),		// Fixed date: Christmas Eve
			new Date(year, 11, 31)		// Fixed date: New Years Eve
		];

		// Pass current date and date array to function
		checkDate(d, closedDates);
	}

	function checkDate(d, closedDates) {

		var openingHour = 10,
			closingHour = 18,
			weekend = [0, 6];

		// Run check time once to see if it is open
		checkTime();

		// Check if time falls in between time span and is not a weekend
		// Change background color if/else
		function checkTime() {
			d = new Date()
			console.log(d);
			if (d.getHours() < openingHour || d.getHours() >= closingHour || d.getDay() === weekend[0] || d.getDay() === weekend[1]) {
				document.body.style.backgroundColor = "red";
			}
			else {
				document.body.style.backgroundColor = "green";
			}
		}
		// Check if current date is equal to a date in the date array
		// Converting to time to be able to compare
		// Change background colour accordingly
		for (i = 0; i < closedDates.length; i++) {
			if (d.getTime() === closedDates[i].getTime()) {
				document.body.style.backgroundColor = "red";
				return
			}
			else {
				// Run script in 30 second intervals to update background reguraly
				setInterval(checkTime, 30000);
				return
			}
		}	

	}

	function calculateClosedDates(year) {
		/* This function calculates and returns an array with date objects of Swedish public holidays
		   Many days are based on when easterDay occurs and therefore I use
		   Gauss Easter Algorithm to calculate when easter day occurs for the current year.

		   It uses constanst M & N that needs to be updated 2099 but this is a non issue.
		   I could include a function that checks which year it is and updates the constants
		   accordingly but I feel it's over ambitious for this script. 

		   Some days are fixed dates and some holidays shift and therefore need to be calculated 
		*/

		// Start Gauss Easter Algorithm
		// https://en.wikipedia.org/wiki/Computus#Gauss's_Easter_algorithm

		// Constants, update year 2099 ;)

		var M = 24,
			N = 5,

			a = year % 19,
			b = year % 4,
			c = year % 7,

			d = ((19*a) + M) % 30,
			e = ((2*b) + (4*c) + (6*d) + N) % 7,

			easterDay = 22 + d + e,

		// Easter Month as default March
			easterMonth = 2;

		// If easterDay is greater than 31, take value minus 31
		// Set month to April
		if (easterDay > 31) {
			easterDay = easterDay - 31;
			easterMonth = 3;
		}

		// Exceptions to formula

		// If easterDay is 26 and easterMonth is April
		// set date a week earlier
		if (easterDay == 26 && easterMonth == 4) {
			easterDay = easterDay - 7;
		}

		// If easterDay is 25, easterMonth is April, d is 28, e is 6 and a is greater than 10
		// set date a week earlier
		if (easterDay == 25 && easterMonth == 4 && d == 28 && e == 6 && a > 10) {
			easterDay = easterDay - 7;
		}
		// End Gauss Easter Formula

		var maundyThursday 	= new Date(year, easterMonth, easterDay-3),				// Will always occur on the first thursday before easter day
			goodFriday 		= new Date(year, easterMonth, easterDay-2),				// Will always occur on the first friday before easter day
			easter 			= new Date(year, easterMonth, easterDay),				// Value from Gauss
			easterMonday 	= new Date(year, easterMonth, easterDay+1),				// Will always occur on the first monday after easter day
			ascension 		= new Date(year, easterMonth, ((easterDay+4) + 35)), 	// Get the next thursday from easter day + 35 days (5 weeks)
			pingstAfton 	= new Date(year, easterMonth, easterDay+48),			// 7 Weeks - 1 day after easter day
			pingstDagen 	= new Date(year, easterMonth, easterDay+49),			// 7 Weeks after easter day	
			midsommarAfton  = getSpecificDay(5, new Date(year, 5, 20)), 			// From start date, find first friday
			midsommarDagen  = getSpecificDay(6, new Date(year, 5, 20)),				// From start date, find first saturday
			allSaintsEve 	= getSpecificDay(5, new Date(year, 9, 30)), 			// From start date, find first friday
			allSaintsDay 	= getSpecificDay(6, new Date(year, 9, 31));				// From start date, find first saturday

		function getSpecificDay(holiday, startDate) {
			// Get which specific day that the holiday (friday, saturday) is and then get the start date of the period 
			var calculatedDate = startDate;
			calculatedDate.setDate(startDate.getDate() + (holiday - startDate.getDay() % 7));

			return calculatedDate;
		}
																		
		return [maundyThursday, goodFriday, easter, easterMonday, ascension, pingstAfton, pingstDagen, midsommarAfton, midsommarDagen, allSaintsEve, allSaintsDay];
	}

Answer 1

Here are a few suggestions..

• The Easter day calculation should be in its own function.
• I would probably put this as a single one liner arrow function (and move it up a few lines) const getSpecificDay = (h, s) => s.setDate(s.getDate() + (h - s.getDay() % 7));. That just looks cleaner IMO.
• You should combine closedDates() and calculateClosedDates() since they're basically both doing the same thing.

Instead of checking the hour every 30 seconds, you're better off calculating the number of milliseconds until the next hour and start checking at that time every hour.

var d = new Date()
d.setHours(d.getHours()+1, 0, 0, 0)
var nextHour = d.getTime() - new Date().getTime();

setTimeout(()=>{
    checkTime();
    setInterval(checkTime, 60*60*1000);
}, nextHour);

Aside from a few formatting things it looks pretty good man.

Answer 2

Never mix information and code

I have a fundamental problem with your code.

Existing data

The calculations for Easter dates looks complex, too complex to trust there are not typos or errors in the code. It would be dangerous to release this service without first testing it over the period you expect this application to provide its service. Hence you should already have a list of Easter holiday dates available to do the test.

Why complicate when you have that data at hand already?

Changing information should not mean changing code

As nothing is set in stone and thus it is highly likely that you will need to make changes to things like open and close times, add exceptions to weekends, amend public holiday times. As you have hard coded all possible information that is subject to possible change changes will require a programmer, and a full test cycle.

Changes cost time and money, and if you don't have a good contract, that cost may be yours to bear, or worse that time may come when you have none to spare.

Deligate responsibility

• You are a coder, you write code.
• The trader runs a trade, manages opening times and does not write code.
• Many people set holidays, they provide services regarding this information

I would highly stress that this type of service (calculating trading hours) be up to the client (the trader) and that you (the coder) provide an interface (server based) for the client to make changes as needed.

That you outsource the calculation of holidays to an API (Example. Randomly selected Holiday API from a google search)

You do not get involved in changing holidays, and trading hours, you provide the means for those that do to provide the information that your code needs to change the background color.

Rewriting your app

The rewrite is an example only, untested, and data copied and unverified. Holidays, trading times and days are as separate data objects that could be delivered as JSON. They are required for the code to load.

To simplify holiday checks, dates are converted to days of the year.

Now your timed function need only do the following.

document.body.classList[tradingInfo.isClosed ? "add" : "remove"]("bg-color--trading-closed");

The code

//=============================================
// Helpers and conversion code
const msInHour = 100 * 60 * 60;    
const msInDay = msInHour * 24;
const tradingTimeZone = 8 * msInHour;
const weekDays = {sun: 0, mon: 1, tue: 2, wed: 3, thu: 4, fri: 5, sat: 6}
const dayOfYearFromDate = date => (date.valueOf() + tradingTimeZone) / msInDay | 0;
const dayOfYear = (year, month, day) => dayOfYearFromDate(new Date(year, month, day));
const dayOfYearOfWeekDayNear = (year, weekDayNameShort, month, date) => {
    const dt = new Date(year, month, date);
    return dayOfYearFromDate(
        dt.setDate(dt.getDate() + (weekDays[weekDayNameShort] - dt.getDay() % 7))
    );
}
const holidaysDayOfYear = (year) => holidays.map(monthDay => dayOfYear(year,...monthDay));
const easterHolidays = (year) => {
    const day = dayOfYear(year, ...easterInfo[year]);
    return [
        ...easterInfo.offsetDays.map(offset => day + offset),
        ...easterInfo.weekDayNear.map(dayNear => dayOfYearOfWeekDayNear(year,...dayNear)),
    ];
}

/* Requires holiday info and trading data  before this can be run and used */

// the object that converts data to isClosed 
const tradingInfo = {
    closed {
        hours: weeklyTradingClosed.hours, // 24hr ranges [from, to]
        days: weeklyTradingClosed.daysOfWeek, // index from 0 sun to 6 sat
        holidays : [ // array of days of the year
            ...easterHolidays(new Date().getFullYear()), 
            ...holidaysDayOfYear(new Date().getFullYear())
        ], 
    },
    get isClosed() {
        const date = new Date();
        const hour = date.getHour();
        const dayOfWeek = date.getDay();
        const dayOfYear = dayOfYearFromDate(date);
        return tradingInfo.closed.hour.some(hours => hour >= hours[0] &&  hour <= hours[1]) ||
            tradingInfo.closed.days.some(day => day = dayOfWeek) || 
            tradingInfo.closed.holidays.includes(dayOfYear);
    },     
};

Information required

// sources
// https://codereview.stackexchange.com/q/211317/120556
const weeklyTradingClosed = {hours : [[0, 10], [18, 24]], daysOfWeek : [0,6]}
const holidays = [
    [0, 1],     // New Years day
    [0, 5],     // Twelfth Night
    [0, 6],     // Epiphany
    [3, 30],    // Walpurgis Night
    [4, 1],     // International Workers' Day
    [5, 6],     // Swedish National Day
    [11, 24],   // Christmas Eve
    [11, 25],   // Christmas Day
    [11, 26],   // Christmas Eve
    [11, 31],   // New Years Eve
};   

Easter info for next 20 years

You don't need a complicated formula, we know the dates already, use that information.

// sources
// https://codereview.stackexchange.com/q/211317/120556
// https://en.wikipedia.org/wiki/List_of_dates_for_Easter  Using western dates.
const easterInfo = { 
    offsetDays : [-3, -2, 0, 1, 39, 48, 49],   
    weekdayNear : [["fri", 5, 20], ["sat", 5, 20], ["fri", 9, 30], ["sat", 9, 31]],
    "2019": [3, 21],
    "2020": [3, 12],
    "2021": [3, 4],
    "2022": [3, 17],
    "2023": [3, 9],
    "2024": [2, 31],
    "2025": [3, 20],
    "2026": [3, 5,],
    "2027": [2, 28],
    "2028": [3, 16],
    "2029": [3, 1],
    "2030": [3, 21],
    "2031": [3, 13],
    "2032": [2, 28],
    "2033": [3, 17],
    "2034": [3, 9],
    "2035": [2, 25],
    "2036": [3, 13],
    "2037": [3, 5],
    "2038": [3, 2],
    "2039": [3, 1],
};

Answer 3

Remove UI code

if (d.getHours() < openingHour || d.getHours() >= closingHour || d.getDay() === weekend[0] || d.getDay() === weekend[1]) {
            document.body.style.backgroundColor = "red";
        }
        else {
            document.body.style.backgroundColor = "green";
        }

Mixing UI code with what should be pure holiday calculation code is the worse thing I see for the long term health of your application. Instead client code (the UI) should call an object method or property but shall not return a color because that's UI business.

Think about it - why would my calculator assume anything about why I'm doing a calculation or how I will use it. When the calculation output is "green" instead of true or false (is what I suspect you want) then that calculator is now useless for anything except that exact, lone thing. The code is not reusable.

---

The comments read the literal code. You should assume the reader knows how to read code and then have comments that tell "why" or "what." Kudos for referencing the algorithm source.

Bad comments:

//Create new date object
var d = new Date();
. . .
// If easterDay is greater than 31, take value minus 31
// Set month to April
if (easterDay > 31) {
   easterDay = easterDay - 31;
   easterMonth = 3;
}
. . .
// Pass current date and date array to function
checkDate(d, closedDates);

OK comments:

// Exceptions to formula
. . .
// Run check time once to see if it is open
    checkTime();

---

Should be unnecessary comments:

// Easter Month as default March
        easterMonth = 2;

Define constants for the months

---

Make closedDates an object then you don't have to memorize the order in the array and induce coding errors.

var closedDates = [
        new Date(year, 0, 1),       // Fixed date: New Years day
        new Date(year, 0, 5),       // Fixed date: Twelfth Night
        new Date(year, 0, 6),       // Fixed date: Epiphany
        new Date(closedDates[0]),   // Thursday

var closedDates = {
        "NewYears" : new Date(year, 0, 1),
        "TwelfthNight" : new Date(year, 0, 5),
        "Epiphany" : new Date(year, 0, 6),
        "Thursday" : new Date(closedDates[0]), 

A weekday should not be in the holidays structure. Make a 'weekdays' structure instead. If you meant 'maundyThursday' I suspect that making an object in the first place would have avoided the commenting error.

---

Naming

Name things to reflect the subject domain not the code implementation details. Sometimes (often?) unnecessary comments - see above - can suggest appropriate names.

closedDates are not dates, they are holidays. And does it really matter to the closedDates user if they are closed or not? Anyway closedDates also includes open dates and so the name is more wrong. What does open and closed mean anyway?

Making day & month constants will make the whole thing far more understandable, significantly prevent errors, and otherwise unnecessary comments.

Rethink all the function names. For example checkDate is meaningless. I know how hard that is; 30 character names is no so good either so sometimes you just have to settle for good function comments.