patternjavascriptMinor
Optimizing a JavaScript snippet for portability and readability
Viewed 0 times
snippetjavascriptreadabilityoptimizingforandportability
Problem
We have an in-house time-tracking application at the office.
We, the employees, can view an overview of our hours on a web page. This web page renders a table, with worked hours per day in table cells, which is styled as a monthly calendar. I wrote a small script which I can copy-paste in my console to get a total of hours worked too much for that month.
A relevant snippet of the HTML.
The JavaScript snippet.
Any thoughts on how to make the script more compact or elegant, while maybe improving readability?
We, the employees, can view an overview of our hours on a web page. This web page renders a table, with worked hours per day in table cells, which is styled as a monthly calendar. I wrote a small script which I can copy-paste in my console to get a total of hours worked too much for that month.
A relevant snippet of the HTML.
10
08:33
11
08:25
The JavaScript snippet.
var elementsToParse = $(".link").find("font:not(:empty)");
var totalMinutes = 0;
$.each(elementsToParse, function(index, element) {
var text = $(element).text();
var hours = parseInt(text.substr(1, 2));
var minutes = parseInt(text.substr(3, 2));
var elementTotal = ((hours * 60) + minutes) - 480;
totalMinutes += elementTotal;
});
alert('Overtime so far: ' + totalMinutes + ' minutes');Any thoughts on how to make the script more compact or elegant, while maybe improving readability?
Solution
I would make it less compact to improve readability. Leave it up to your build process (minifier) to make things compact. Your goal should be to make things understandable.
Use regular expressions
In cases like this, where you have a strict format, I love using regular expressions:
This makes it clear that you expect a date to consist of two digits a colon, and two more digits. As a bonus, let everyone know it's a constant with JSDoc and common coding conventions.
Define functions
Short modular code is easy to read:
Here, the
Put it all together
Putting it together, your actual function does become more compact and more readable because tangental functions are extracted:
Protecting global, enabling compilation
Right now we are exposing a variety of variables and functions on the global object. This means that anyone could call
Use regular expressions
In cases like this, where you have a strict format, I love using regular expressions:
/** @const */
var TIME_FORMAT = /(\d{2}):(\d{2})/;
...
"10:30".match(TIME_FORMAT) // == ["10:30", "10", "30"]This makes it clear that you expect a date to consist of two digits a colon, and two more digits. As a bonus, let everyone know it's a constant with JSDoc and common coding conventions.
Define functions
Short modular code is easy to read:
/**
* Parses a time string and returns the hours and minutes.
*/
function parseTime(text) {
var match = String(text).match(TIME_FORMAT);
if (match === null) {
throw new Error('Time "' + text + '" could not be parsed.');
}
return {
hours: Number(match[1]),
minutes: Number(match[2])
};
}
/**
* Gets the total minutes of overtime based on time worked and number of working hours
* in a day.
*/
function calculateOvertime(parsedTime, workingHours) {
var minutesWorked = parsedTime.hours * 60 + parsedTime.minutes;
var minutesRequired = workingHours * 60;
return minutesWorked - minutesRequired;
}Here, the
parseTime parses the time, returning an object, and calculateOvertime calculates the overtime.Put it all together
Putting it together, your actual function does become more compact and more readable because tangental functions are extracted:
/**
* Gets the total minutes of overtime from a tracking table.
*
* @param selector A CSS2.1 selector, DOM element or jQuery object pointing to the
* tracking table.
*/
function getOvertimeFromTable(selector, dailyWorkingHours) {
var totalMinutesOfOvertime = 0;
var $dailyTimes = $(selector).find("font:not(:empty)");
$dailyTimes.each(function () {
var parsedTime = parseTime($(this).text());
var minutesOfOvertime = calculateOvertime(parsedTime, dailyWorkingHours);
totalMinutesOfOvertime += minutesOfOvertime;
});
}
alert('Overtime so far: ' + getOvertimeFromTable('.links', 8) + ' minutes');Protecting global, enabling compilation
Right now we are exposing a variety of variables and functions on the global object. This means that anyone could call
parseTime or read a similar function. It also means a minifier can't rename the function to a shorter name. Wrapping everything in a closure will be the final step for making sure your output code is more compact and your source needn't be:/**
* Alerts the number of hours of overtime you have accumulated so far in this pay cycle,
* assuming a standard 8-hour day.
*/
(function () {
/* Your code here */
}());Code Snippets
/** @const */
var TIME_FORMAT = /(\d{2}):(\d{2})/;
...
"10:30".match(TIME_FORMAT) // == ["10:30", "10", "30"]/**
* Parses a time string and returns the hours and minutes.
*/
function parseTime(text) {
var match = String(text).match(TIME_FORMAT);
if (match === null) {
throw new Error('Time "' + text + '" could not be parsed.');
}
return {
hours: Number(match[1]),
minutes: Number(match[2])
};
}
/**
* Gets the total minutes of overtime based on time worked and number of working hours
* in a day.
*/
function calculateOvertime(parsedTime, workingHours) {
var minutesWorked = parsedTime.hours * 60 + parsedTime.minutes;
var minutesRequired = workingHours * 60;
return minutesWorked - minutesRequired;
}/**
* Gets the total minutes of overtime from a tracking table.
*
* @param selector A CSS2.1 selector, DOM element or jQuery object pointing to the
* tracking table.
*/
function getOvertimeFromTable(selector, dailyWorkingHours) {
var totalMinutesOfOvertime = 0;
var $dailyTimes = $(selector).find("font:not(:empty)");
$dailyTimes.each(function () {
var parsedTime = parseTime($(this).text());
var minutesOfOvertime = calculateOvertime(parsedTime, dailyWorkingHours);
totalMinutesOfOvertime += minutesOfOvertime;
});
}
alert('Overtime so far: ' + getOvertimeFromTable('.links', 8) + ' minutes');/**
* Alerts the number of hours of overtime you have accumulated so far in this pay cycle,
* assuming a standard 8-hour day.
*/
(function () {
/* Your code here */
}());Context
StackExchange Code Review Q#13705, answer score: 3
Revisions (0)
No revisions yet.