patternjavascriptMinorCanonical
Prorated Refund Calculator (v2)
Viewed 0 times
proratedcalculatorrefund
Problem
This is a follow-up question to original Prorated Refund Calculator. Here is the summary of the changes I made since last, thanks to the good answers and some study of my own.
Is there anything else that I could improve before I share this with my colleagues?
`
Prorated Refund Calculator
body {
font-family: Calibri, Arial, sans-serif;
font-size: 1.0em;
}
h1 {
font-size: 1.2em;
}
Prorated Refund Calculator
Input into the following fields and press Calculate.
Enter date format as MM/DD/YYYY
Product Purchase Date:
Contract Purchase Date:
Purchase Price:
Term (in years):
Cancel Date:
Amount paid in claims:
Grace period (in days):
Calculate
/**
* Main function called by HTML form.
* Validates input values, then performs calculations
* and adds the result into the HTML document to display to the user.
*/
function calculateProratedRefund() {
"use strict";
var productPurchaseDate = new Date(document.getElementById("productPurchaseDate").value);
var contractPurchaseDate = new Date(document.getElementById("contractPurchaseDate").value);
var purchasePrice = parseFloat(document.getElementById("purchasePrice").value).toFixed(2);
var termInYears = parseInt(document.getElementById("termInYears").value, 10);
var cancelDate = new Date(document.getElementById("cancelDate").value);
var amtPa
- Did away with bleeding-edge ECMAScript 6 code that was causing compatibility issues with some browsers.
- Added more input validation.
- Added a utility function to format dates to MM/DD/YYYY format.
- Input values are now obtained using
getElementByIdinstead of getting them from a numbered list.
- Output is appended into the existing page, rather than updating placeholder HTML tags.
- Documentation throughout.
- Output is more comprehensive and user-friendly.
Is there anything else that I could improve before I share this with my colleagues?
`
Prorated Refund Calculator
body {
font-family: Calibri, Arial, sans-serif;
font-size: 1.0em;
}
h1 {
font-size: 1.2em;
}
Prorated Refund Calculator
Input into the following fields and press Calculate.
Enter date format as MM/DD/YYYY
Product Purchase Date:
Contract Purchase Date:
Purchase Price:
Term (in years):
Cancel Date:
Amount paid in claims:
Grace period (in days):
Calculate
/**
* Main function called by HTML form.
* Validates input values, then performs calculations
* and adds the result into the HTML document to display to the user.
*/
function calculateProratedRefund() {
"use strict";
var productPurchaseDate = new Date(document.getElementById("productPurchaseDate").value);
var contractPurchaseDate = new Date(document.getElementById("contractPurchaseDate").value);
var purchasePrice = parseFloat(document.getElementById("purchasePrice").value).toFixed(2);
var termInYears = parseInt(document.getElementById("termInYears").value, 10);
var cancelDate = new Date(document.getElementById("cancelDate").value);
var amtPa
Solution
Your code is really clean, but it could be improved in places:
-
You could consider
-
Your closing `
-
The naming behind
-
Rather than repeating
like:
-
Your usage of brackets around math operations is inconsistent:
I'd suggest sticking to the same usage throughout, the brackets seem clearer, but often aren't required.
-
Rather than testing
Then, you can simply
into:
Note that, in your code, you test against negatively (
I'm not familiar with the level of JavaScript expertise your colleagues have, but, this code is well written, the code is consistent, and it contains descriptive
-
You could consider
placeholder attributes in the input fields on your HTML to display the format of the data that should be entered.-
Your closing `
tag isn't indented on the same level as the opening tag, it should be.
-
Style-wise, It's easier on people if you can align all your s to the same vertical values, rather than based on the length of the description.
-
You have extraneous whitespace in some of your brackets: if ( finalRefund -
The naming behind
outputPar could be improved; outputParagraph: for example.-
Rather than repeating
outputPar.appendChild() over and over, move it to an array, and do it in a loop:outputPar.appendChild(document.createTextNode("Product Purchase date: " + formatDate(productPurchaseDate)));
outputPar.appendChild(document.createElement("br"));like:
var contentToBuild = [
"Product Purchase date: " + formatDate(productPurchaseDate),
// More here
];
contentToBuild.forEach(function(element){
outputParagraph.appendChild(document.createTextNode(element));
outputParagraph.appendChild(document.createElement("br"));
});-
Your usage of brackets around math operations is inconsistent:
var msPerDay = (1000 * 60 * 60 * 24);
var termLeft = 1.0 - termUsed;I'd suggest sticking to the same usage throughout, the brackets seem clearer, but often aren't required.
-
isValidDate() could use some re-arranging.function isValidDate(date) {
"use strict";
if ( Object.prototype.toString.call(date) !== "[object Date]" ) {
return false;
}
else if ( isNaN(date.getTime()) ) {
return false;
}
else {
return true;
}
}Rather than testing
isNaN(date.getTime()) second, test it first, and remove it from the if-else block.Then, you can simply
return the boolean value ofObject.prototype.toString.call(date) !== "[object Date]"into:
function isValidDate(date) {
"use strict";
if (isNaN(date.getTime())) {
return false;
}
return (Object.prototype.toString.call(date) === "[object Date]");
}Note that, in your code, you test against negatively (
!==) the variable, and return false. It'd be easier, in future, if you test against positively (===) and returned true.I'm not familiar with the level of JavaScript expertise your colleagues have, but, this code is well written, the code is consistent, and it contains descriptive
@params and @returns. Good Work!Code Snippets
if (finalRefund < 0.0)outputPar.appendChild(document.createTextNode("Product Purchase date: " + formatDate(productPurchaseDate)));
outputPar.appendChild(document.createElement("br"));var contentToBuild = [
"Product Purchase date: " + formatDate(productPurchaseDate),
// More here
];
contentToBuild.forEach(function(element){
outputParagraph.appendChild(document.createTextNode(element));
outputParagraph.appendChild(document.createElement("br"));
});var msPerDay = (1000 * 60 * 60 * 24);
var termLeft = 1.0 - termUsed;function isValidDate(date) {
"use strict";
if ( Object.prototype.toString.call(date) !== "[object Date]" ) {
return false;
}
else if ( isNaN(date.getTime()) ) {
return false;
}
else {
return true;
}
}Context
StackExchange Code Review Q#96842, answer score: 7
Revisions (0)
No revisions yet.