11
\$\begingroup\$

I wrote this little piece of script for my workplace (insurance, etc.) to help our admins, as a light and simple file I can just email to my colleagues.

The purpose of it is to calculate a prorated refund in case of a policy holder cancelling it. Basically a policy's purchase value "ticks down" gradually after a certain grace period (calculated in days), and any claims paid against the policy are then deducted from the remaining amount.

I tested for a number of valid values and it works good. I also tested for invalid input and it returns Invalid Date if either of the dates can't be reconciled to a valid Date() value, or the calculations return NaN, which is fine with me but I'm open to more elegant solutions.

<!DOCTYPE html>
<html>
<head>
  <title>Prorated Refund Calculator</title>
  <style>
  body {
    font-family: Consolas, Monaco, monospace;
    font-size: 1.0em;
  }
  h1 {
    font-size: 1.2em;
  }
</style>
</head>
<body>
<h1>Prorated Refund Calculator</h1>
<p>Input into the following fields and press Calculate.</p>
<form>
  <ul>
    <li>Purchase Date: <input type="text" name="input"></li>
    <li>Purchase Price: <input type="text" name="input"></li>
    <li>Term (in years): <input type="text" name="input" value="10"></li>
    <li>Cancel Date: <input type="text" name="input"></li>
    <li>Amount paid in claims: <input type="text" name="input" value="0"></li>
    <li>Grace period (in days): <input type="text" name="input" value="60"></li>
  </ul>
</form> 

<button onclick="CalculateProratedRefund()">Calculate</button>

<p id="purchaseDateOutput"></p>
<p id="cancelDateOutput"></p>
<p id="daysElapsedOutput"></p>
<p id="totalDaysOutput"></p>
<p id="percentRefundOutput"></p>
<p id="purchasePriceOutput"></p>
<p id="proratedRefundOutput"></p>
<p id="amtPaidInClaimsOutput"></p>
<p id="finalRefundAmountOutput"></p>

<script>
function CalculateProratedRefund() {
    "use strict";
    let input= document.getElementsByName("input");

    // get initial values from HTMLCollection
    let purchaseDate = new Date(input[0].value);
    let purchasePrice = input[1].value;
    let termInYears = input[2].value;
    let cancelDate = new Date(input[3].value);
    let amtPaidInClaims = input[4].value;
    let gracePeriodInDays = input[5].value;

    // calculations
    let daysElapsed = Math.floor(( Date.parse(cancelDate) - Date.parse(purchaseDate) ) / 86400000);
    let totalDays = termInYears * 365;
    let percentUsed;
    if (daysElapsed <= gracePeriodInDays) {
        percentUsed = 0.0;
    } else {
        percentUsed = daysElapsed / totalDays;
    }
    let percentLeft = 1.0 - percentUsed;
    let refundPercent = purchasePrice * percentLeft;
    let proratedRefund = (purchasePrice * refundPercent) / 100;
    let finalRefund = proratedRefund - amtPaidInClaims;

    // add results to HTML output
    document.getElementById("purchaseDateOutput").innerHTML = "Purchase date: " + purchaseDate;
    document.getElementById("cancelDateOutput").innerHTML = "Cancel date: " + cancelDate;
    document.getElementById("daysElapsedOutput").innerHTML = "Days elapsed: " + daysElapsed;
    document.getElementById("totalDaysOutput").innerHTML = "Total days: " + totalDays;
    document.getElementById("percentRefundOutput").innerHTML = "Refund percent: " + percentLeft * 100 + " %";
    document.getElementById("purchasePriceOutput").innerHTML = "Purchase price: $ " + purchasePrice;
    document.getElementById("proratedRefundOutput").innerHTML = "Prorated refund: $ " + proratedRefund;
    document.getElementById("amtPaidInClaimsOutput").innerHTML = "Paid in claims: $ " + amtPaidInClaims;
    document.getElementById("finalRefundAmountOutput").innerHTML = "Final refund: $ " + finalRefund;
}
</script>

</body>
</html>

\$\endgroup\$
  • \$\begingroup\$ Do you have sample values to test with? It does not like my input. \$\endgroup\$ – Mast May 25 '15 at 20:20
  • \$\begingroup\$ Try using 04/01/2014 as purchase date, 05/25/2015 as cancel date, 100 as purchase price \$\endgroup\$ – Phrancis May 25 '15 at 20:28
  • \$\begingroup\$ What browser have you gotten this to run in? Just tried the snippet in Chrome, Safari and Firefox and neither of them wants any part of it. Specifically because let isn't supported (it practically isn't supported anywhere, really, which is why I'm curious as to how you're running it). \$\endgroup\$ – Flambino May 25 '15 at 23:55
  • \$\begingroup\$ @Flambino made it with IE11, worked fine (albeit a warning about cache I had to dismiss). Just tested snippet in Chrome for Mac and also works fine \$\endgroup\$ – Phrancis May 25 '15 at 23:56
  • \$\begingroup\$ Here is a screenshot \$\endgroup\$ – Phrancis May 25 '15 at 23:58
6
\$\begingroup\$

Bug in the calculation

This part looks bugged:

let percentLeft = 1.0 - percentUsed;
let refundPercent = purchasePrice * percentLeft;
let proratedRefund = (purchasePrice * refundPercent) / 100;

I believe it should be:

let percentLeft = 1.0 - percentUsed;
let proratedRefund = purchasePrice * percentLeft;

Because, multiplying percentLeft with price and dividing by 100 makes no sense, effectively: purchasePrice * percentLeft * purchasePrice / 100. What does it mean to have purchase price squared? Why divide something by 100 that's already a rate.

I saw in your comments to other users that you used 100 as the example price. In which case you get the correct value by accident, and I guess that's the reason you didn't notice the problem.

Terminology

Some terms are misused, which makes the code (and your intentions) a bit confusing, and may also lead to bugs.

The word percent usually means a value between 0 and 100. But you're using these variables really in the sense of rates, as they are values between 0 and 1.

Magic number 86400000

Are you sure there are enough zeros in there? Of you copy pasted (as opposed to copy-typed by hand), then you're probably sure. But your readers cannot know that. If you write as 60*60*24*1000 are you sure there are no typing errors? I think so.

Parsing dates

It seems you're parsing dates twice. Once with new Date and then again with Date.parse. Probably once would be enough.

A related usability issue, the correct date format to enter might not be obvious to all readers. It would be nice to add a placeholder text in the input box as a hint.

\$\endgroup\$
  • \$\begingroup\$ Wow, how did I miss that! That will teach me to do more sanity checks with data. Thanks a lot! \$\endgroup\$ – Phrancis May 26 '15 at 16:33
3
\$\begingroup\$

Janos gave a great answer. I only have a few things to add.

As discussed in the comments, using the latest JavaScript syntax is a bit problematic. At least for me, since none of my installed browsers would run the code at all and instead considered it a syntax error due to your use of let.

Now, I completely understand your desire to use the latest stuff, and if you know your target audience (or their browsers, at least) can run it, you're free to do so. But I do wonder if it's worth it.

To get your code running in all my browsers, all I needed was to use var instead of let. Yes, I'm aware this changes the semantics - but it also makes the code run. Actually, I'm fairly confident it'll even run in browsers that are literally a decade old.

Besides, while let is semantically different from var, in this particular piece of code they're interchangeable: All your let variables are local to the function, just as they would be with var. A let-declared variable is local to its block, but the only two blocks you have (the if/else) don't declare any new variables; they refer back to a variable in the function's scope.

So I can't actually find any reason to use let at all, to be honest. At best, it makes no difference whatsoever, and at worst it's treated as a syntax error, and the code doesn't run at all.

Other stuff:

  • You have a couple of magic numbers for doing the date arithmetic. Not a huge concern, except that one of them (365) perhaps isn't accurate due to leap years. Of course, if that's how a year is defined in your policies, then it's accurate. But if a 10-year term means date-to-date, then it's inaccurate. For anything financial, I'd triple-check.

  • All your output elements are uniquely named, but your inputs are collectively named. I'd do almost the exact opposite: Give the inputs unique IDs, and generate the output elements in their entirety instead of updating existing ones. If nothing else, it'd reduce the implicit coupling to the HTML, simply because you wouldn't be relying on the inputs being in the same order they're in right now. If I were to say that the "grace period" input should swap places with the "term in years" input, the code would still run - but produce very, very wrong results.
    Meanwhile, the output elements could all be created with document.createElement, since they're all plain paragraph elements. Not saying you have to do that, but again it'd reduce coupling.

  • I'd use <label> elements for the inputs, but skip the <ul>. Semantically, the inputs aren't a "list", per se. Well, the argument could be made, but... eh. Any form can be said to be a list of fields, so the distinction's kinda pointless.
    It's also a little ironic that it's an unordered list, given that the code treats it as very much ordered :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.