HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinorCanonical

Prorated Refund Calculator (v2)

Submitted by: @import:stackexchange-codereview··
0
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.

  • 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 getElementById instead 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 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 of

Object.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.