Skip to content

Commit

Permalink
Optimize the HoursTransformer (#784)
Browse files Browse the repository at this point in the history
Improve the HoursTransformer

The clonedeep unnecessarily increased the computation time. Removing it improves performance by about 22%. This PR also improves the clarity of the code by making it more functional and making the data models easier to understand.

J=SLAP-1289
TEST=manual, auto

Smoke test the openStatus formatter and the hoursList formatter which both rely on this code. Add unit tests. Benchmark the code by running the function 1000 times and comparing the time in milliseconds before and after.
  • Loading branch information
cea2aj committed May 19, 2021
1 parent 8866b78 commit 78a6b96
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 49 deletions.
126 changes: 77 additions & 49 deletions static/js/hours/transformer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import clonedeep from 'lodash.clonedeep';
import { DayNames } from './constants.js';
import Hours from './models/hours.js';
import { OpenStatusTypes } from './open-status/constants.js';
Expand Down Expand Up @@ -177,6 +176,8 @@ export default class HoursTransformer {
}

/**
* Transforms the hours data into the shape needed by the front-end
*
* @param {Object} days e.g.
* {
* monday: {
Expand All @@ -193,43 +194,28 @@ export default class HoursTransformer {
* @returns {Object[]}
*/
static _formatHoursForAnswers(days, timezone) {
const formattedDays = clonedeep(days);
const daysOfWeek = Object.values(DayNames);
const holidayHours = formattedDays.holidayHours || [];
for (let day in formattedDays) {
if (day === 'holidayHours' || day === 'reopenDate') {
delete formattedDays[day];
} else {
return Object.entries(days)
.filter(([day]) => day !== 'holidayHours' && day !== 'reopenDate')
.reduce((formattedDays, [day, dayInfo]) => {
const currentDayName = day.toUpperCase();
const numberTimezone = this._convertTimezoneToNumber(timezone);
const userDateToEntityDate = this._getDateWithUTCOffset(numberTimezone);
const dayNameToDate = this._getNextDayOfWeek(userDateToEntityDate, daysOfWeek.indexOf(currentDayName));

for (let holiday of holidayHours) {
let holidayDate = new Date(holiday.date + 'T00:00:00.000');
if (dayNameToDate.toDateString() == holidayDate.toDateString()) {
holiday.intervals = this._formatIntervals(holiday.openIntervals);
formattedDays[day].dailyHolidayHours = holiday;
}
}

formattedDays[day].day = day.toUpperCase();

let intervals = formattedDays[day].openIntervals;
if (intervals) {
for (let interval of intervals) {
for (let period in interval) {
interval[period] = parseInt(interval[period].replace(':', ''));
}
}
} else {
formattedDays[day].openIntervals = [];
const userDate = this._getDateWithUTCOffset(numberTimezone);
const nextDayOfWeek = this._getNextDayOfWeek(userDate, daysOfWeek.indexOf(currentDayName));
const dailyHolidayHours = this._getDailyHolidayHours(days.holidayHours, nextDayOfWeek);
const openIntervals = (dayInfo.openIntervals || []).map(this._formatInterval);

const formattedDay = {
day: currentDayName,
...dailyHolidayHours && { dailyHolidayHours },
openIntervals: openIntervals,
intervals: openIntervals,
isClosed: dayInfo.isClosed
}
formattedDays[day].intervals = formattedDays[day].openIntervals;
}
}

return Object.values(formattedDays);
formattedDays.push(formattedDay);
return formattedDays;
}, []);
}

/**
Expand Down Expand Up @@ -272,24 +258,66 @@ export default class HoursTransformer {
}

/**
* Returns the hours intervals array with hours parsed into a number
* e.g. "09:00" turning into 900.
* @param {Object[]} intervals
* @param {string} intervals[].start start time like "09:00"
* @param {string} intervals[].end end time like "17:00"
* @returns {Object[]}
* @typedef HolidayHourInfo
* @type {Object}
* @property {string} date the date of the holiday
* @property {boolean} isClosed indicates whether or not the location is closed
* @property {boolean} isRegularHours indicates whether or not the location has regular hours on the holiday
* @property {OpenInterval[]} openIntervals intervals of time when the location is open
*/
static _formatIntervals(intervals) {
if (!intervals) {
return [];
}
let formatted = Array.from(intervals);
for (let interval of formatted) {
for (let period in interval) {
interval[period] = parseInt(interval[period].replace(':', ''));
}

/**
* @typedef OpenInterval
* @type {Object}
* @property {string} start start time, e.g. '03:00'
* @property {string} end end time, e.g '09:00'
*/

/**
* Gets the holiday hours for the provided date given a HoldiayHourInfo list
* @param {HolidayHourInfo[]} holidayHours
* @param {Date} date
* @returns {Object|null}
*/
static _getDailyHolidayHours (holidayHours = [], date) {
const holidayHoursForDate = holidayHours.find(holiday => {
const holidayDate = new Date(holiday.date + 'T00:00:00.000');
return date.toDateString() == holidayDate.toDateString();
});

if (!holidayHoursForDate) {
return null;
}
return formatted;

const formattedIntervals = (holidayHoursForDate.openIntervals || []).map(this._formatInterval);

return {
date: holidayHoursForDate.date,
intervals: formattedIntervals,
openIntervals: formattedIntervals,
...holidayHoursForDate.isClosed && { isClosed: true },
...holidayHoursForDate.isRegularHours && { isRegularHours: true }
};
}

/**
* @typedef NumericOpenInterval
* @type {Object}
* @property {number} start start time, e.g. '300'
* @property {number} end end time, e.g '900'
*/

/**
* Returns the hours intervals object with hours parsed into a number
* e.g. "09:00" turning into 900.
* @param {OpenInterval} interval
* @returns {NumericOpenInterval}
*/
static _formatInterval (interval) {
return Object.entries(interval).reduce((formattedInterval, [period, value]) => {
formattedInterval[period] = parseInt(value.replace(':', ''));
return formattedInterval;
}, {});
}

/**
Expand Down
127 changes: 127 additions & 0 deletions tests/static/js/hours/transformer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { DayNames } from '../../../../static/js/hours/constants';
import HoursTransformer from '../../../../static/js/hours/transformer';

const dateClass = Date;
const mockDateClass = class extends Date {
constructor(date = 'July 21, 2020 12:42:00 GMT-0500') {
super(date);
}
};

describe('formats hours for answers', () => {
beforeEach(() => {
// Mock the date class because holiday hours are only applied if they are for
// dates which are within one week of the current date.
global.Date = mockDateClass;
});

afterEach(() => {
global.Date = dateClass;
})

it('formats hours for multiple days', () => {
const expectedFormattedHours = [
{
day: DayNames.MONDAY,
intervals: [{end: 400, start: 100}],
isClosed: false,
openIntervals: [{end: 400, start: 100}],
},
{
day: DayNames.TUESDAY,
intervals: [{end: 800, start: 100}],
openIntervals: [{end: 800, start: 100}],
isClosed: false
},
{
day: DayNames.WEDNESDAY,
intervals: [],
openIntervals: [],
isClosed: true
}
];
const days = {
monday: {
isClosed: false,
openIntervals: [{ start: '01:00', end: '04:00' }]
},
tuesday: {
isClosed: false,
openIntervals: [{ start: '01:00', end: '08:00' }]
},
wednesday: {
isClosed: true
}
}
const timezoneOffset = '-04:00';
const actualFormattedHours = HoursTransformer._formatHoursForAnswers(days, timezoneOffset);
expect(actualFormattedHours).toEqual(expectedFormattedHours);
});

it('formats hours for multiple days and holiday hours', () => {
const expectedFormattedHours = [
{
day: DayNames.TUESDAY,
intervals: [{end: 800, start: 100}],
openIntervals: [{end: 800, start: 100}],
isClosed: false,
dailyHolidayHours: {
date: '2020-07-21',
intervals: [{end: 200, start: 100}],
openIntervals: [
{
end: 200,
start: 100,
},
],
},
},
{
day: DayNames.WEDNESDAY,
intervals: [{end: 1000, start: 100}],
openIntervals: [{end: 1000, start: 100}],
isClosed: false,
dailyHolidayHours: {
date: '2020-07-22',
isClosed: true,
intervals: [],
openIntervals: [],
},
},
{
day: DayNames.THURSDAY,
intervals: [{end: 400, start: 100}],
isClosed: false,
openIntervals: [{end: 400, start: 100}],
dailyHolidayHours: {
date: '2020-07-23',
intervals: [],
openIntervals: [],
isRegularHours: true
},
},
];
const days = {
tuesday: {
isClosed: false,
openIntervals: [{ start: '01:00', end: '08:00' }]
},
wednesday: {
isClosed: false,
openIntervals: [{ start: '01:00', end: '10:00' }]
},
thursday: {
isClosed: false,
openIntervals: [{ start: '01:00', end: '04:00' }]
},
holidayHours: [
{ date: '2020-07-21', openIntervals: [{ start: '01:00', end: '02:00' }] },
{ date: '2020-07-22', isClosed: true },
{ date: '2020-07-23', isRegularHours: true }
]
}
const timezoneOffset = '-04:00';
const actualFormattedHours = HoursTransformer._formatHoursForAnswers(days, timezoneOffset);
expect(actualFormattedHours).toEqual(expectedFormattedHours);
});
});

0 comments on commit 78a6b96

Please sign in to comment.