patternjavascriptMinor
Hours:Minutes Addition
Viewed 0 times
hoursadditionminutes
Problem
I wrote this simple page to help at work, and as an exercise to get more familiar with JavaScript.
I'm especially interested in help with style and structure - is this "standard"?
I know the input isn't validated at all - feel free to make suggestions here but this isn't my main concern at this time.
behavior.js:
Simple Hours:Minutes Adder
Enter duration in format hh:mm and select ok, running total at top
text
`
I'm especially interested in help with style and structure - is this "standard"?
I know the input isn't validated at all - feel free to make suggestions here but this isn't my main concern at this time.
behavior.js:
var output_prefix = 'Total duration is: ';
var entries = [];
var list_display = null;
function addDuration() {
var input_text = document.getElementById("input_text");
if (1) { //add validation, input must be hh:mm
var hhmm = input_text.value.split(":");
var input_val = Number(hhmm[0]*60) + Number(hhmm[1]);
entries.push(input_val);
updateTotalDisplay();
var new_node = document.createElement("P");
new_node.appendChild(document.createTextNode(formatTime(input_val)));
list_display.appendChild(new_node);
} else {
alert("not a number");
}
input_text.value = "";
input_text.focus();
}
function popDuration() {
entries.pop();
updateTotalDisplay();
list_display.removeChild(list_display.lastChild);
}
function updateTotalDisplay() {
var total = 0;
for (var i = 0; i=0 && mm
Simple Hours:Minutes Adder
Enter duration in format hh:mm and select ok, running total at top
text
`
Solution
HTML5 style
I'm not sure if there's a standard for HTML formatting.
I think it usually depends on the team.
You might be interested in Google's style guide for HTML.
It seems logical to indent `
It's reasonable to expect that this function will receive already validated number of minutes (0 or positive).
The function could also use better variable names,
and some string operations can be written simpler,
and some a variable can be eliminated:
I'm not sure if there's a standard for HTML formatting.
I think it usually depends on the team.
You might be interested in Google's style guide for HTML.
It seems logical to indent `
and deeper than ,
so instead of:
Simple Hours:Minutes Adder
This would be better:
Simple Hours:Minutes Adder
HTML5 validation
In HTML5, the tag should not have a closing .
The first tag here violates that:
There are some other HTML5 violations, for example missing .
Use a validator to catch these.
JavaScript style
Indenting using two spaces is a very common practice, so that's good.
In general your JavaScript style seems fine, clean, easy to read.
Some minor violations:
- pointless
return; at the end of updateTotalDisplay
total = total + entries[i] can be simplified to total += entries[i]
- instead of setting
totaltimetext to output_prefix + formatTime(total), it would be better to have output_prefix in the HTML document, and only update the changing part, in a dedicated field
Use booleans
Instead of this:
if (1) { //add validation, input must be hh:mm
Better use proper booleans:
if (true) { //add validation, input must be hh:mm
But even better would be to replace the comment with a function call:
function is_valid(hhmm) {
//implement validation, input must be hh:mm
return true;
}
var hhmm = input_text.value.split(":");
if (is_valid(hhmm)) {
Improving the logic
Instead of re-summing the minutes stored in entries,
it would be more efficient to keep track of the running total,
and simply add the newly added number of minutes.
Improving formatTime
Instead of:
if (mm >= 0 && mm < 10) {
It's generally more readable to arrange the terms in increasing order:
if (0 <= mm && mm < 10) {
But actually, the 0 <= mm` seems pointless.It's reasonable to expect that this function will receive already validated number of minutes (0 or positive).
The function could also use better variable names,
and some string operations can be written simpler,
and some a variable can be eliminated:
function formatTime(total_minutes) {
var hours = Math.floor(total_minutes / 60);
var minutes = total_minutes % 60;
if (minutes < 10) {
minutes = '0' + minutes;
}
return hours + ':' + minutes;
}Code Snippets
<!DOCTYPE html>
<html>
<head>
<script src="behavior.js"></script>
</head>
<body>
<h1>Simple Hours:Minutes Adder</h1><!DOCTYPE html>
<html>
<head>
<script src="behavior.js"></script>
</head>
<body>
<h1>Simple Hours:Minutes Adder</h1><input id="input_text" type="text"></input>
<input id="add_button" type="button" value="add">
<input id="pop_button" type="button" value="pop item">if (1) { //add validation, input must be hh:mmif (true) { //add validation, input must be hh:mmContext
StackExchange Code Review Q#115261, answer score: 4
Revisions (0)
No revisions yet.