7
\$\begingroup\$

So I have the following code which works well-ish. Looking for ideas on how to improve it.

public void testItAll() {
    System.err.println(determineRate(226.34, 25.00, 12));
    System.err.println(determineRate(5800.00, 29.00, 31));
    System.err.println(determineRate(4000.00, 25.00, 460));
    System.err.println(determineRate(4000.00, 111.00, 173));
    System.err.println(determineRate(15000.00, 270.00, 60));
    System.err.println(determineRate(7000.00, 154.00, 60));
    System.err.println(determineRate(27002.51, 315.00, 85));
    System.err.println(determineRate(17118.33, 270.14, 73));
    System.err.println(determineRate(5170.71, 143.00, 48));
    System.err.println(determineRate(45297.34, 400.00, 135));
    System.err.println(determineRate(6000.00, 113.00, 60));
    System.err.println(determineRate(23058.98, 219.59, 145));
    System.err.println(determineRate(47475.76, 390.44, 181));
    System.err.println(determineRate(69691.48, 554.00, 180));
    System.err.println(determineRate(39310.00, 725.00, 60));
    System.err.println(determineRate(45000.00, 316.00, 180));
    System.err.println(determineRate(14071.01, 45.00, 220));
    System.err.println(determineRate(16875.00, 198.00, 120));
    System.err.println(determineRate(66080.04, 295.00, 12));
    System.err.println(determineRate(120000.00, 664.00, 120));
    System.err.println(determineRate(58000.00, 387.19, 120));
    System.err.println(determineRate(351213.00, 1993.11, 25));
    System.err.println(determineRate(139500.00, 1000.00, 300));
    System.err.println(determineRate(2400.00, 1875.00, 180));
    System.err.println(determineRate(193155.00, 2002.00, 120));
    System.err.println(determineRate(40800.00, 507.46, 36));
    System.err.println(determineRate(198375.00, 1530.00, 240));
    System.err.println(determineRate(22700.00, 450.00, 60));
    System.err.println(determineRate(20000.00, 622.85, 999));
    System.err.println(determineRate(629999.32, 25.00, 300));
    System.err.println(determineRate(298905.41, 0.01, 360));
    System.err.println(determineRate(329850.00, 2075.00, 240));
    System.err.println(determineRate(21163.58, 322.00, 72));
    System.err.println(determineRate(42342.08, 320.00, 73));
    System.err.println(determineRate(33000.00, 335.00, 120));
    System.err.println(determineRate(21234.64, 207.73, 144));
    System.err.println(determineRate(103400.00, 787.00, 240));
    System.err.println(determineRate(82262.45, 565.00, 120));
    System.err.println(determineRate(73300.00, 610.00, 60));
    System.err.println(determineRate(131948.07, 528.00, 180));
    System.err.println(determineRate(14000.00, 102.00, 180));
    System.err.println(determineRate(74847.00, 610.00, 240));
    System.err.println(determineRate(50000.00, 544.00, 120));
    System.err.println(determineRate(15000.00, 167.00, 120));
    System.err.println(determineRate(741940.00, 4135.00, 60));
    System.err.println(determineRate(62540.34, 999.00, 84));
    System.err.println(determineRate(27277.50, 325.00, 120));
    System.err.println(determineRate(24435.57, 375.00, 72));
    System.err.println(determineRate(3000.00, 104.00, 0));
    System.err.println(determineRate(2927.03, 105.00, 36));
    System.err.println(determineRate(11000.00, 128.00, 120));
    System.err.println(determineRate(8000.00, 180.00, 60));
    System.err.println(determineRate(13459.32, 206.90, 84));
    System.err.println(determineRate(25828.47, 277.00, 145));
    System.err.println(determineRate(19395.36, 250.00, 120));
    System.err.println(determineRate(240000.00, 245058.08, 1));
    System.err.println(determineRate(65148.68, 450.00, 180));
    System.err.println(determineRate(86000.00, 719.00, 120));
    System.err.println(determineRate(34147.83, 298.27, 181));
    System.err.println(determineRate(8230.29, 150.00, 73));
    System.err.println(determineRate(85993.71, 659.54, 360));
    System.err.println(determineRate(110000.00, 515.43, 92));
    System.err.println(determineRate(563000.00, 3555.13, 360));
    System.err.println(determineRate(3000.00, 40.00, 61));
    System.err.println(determineRate(440.67, 30.00, 312));
    System.err.println(determineRate(457.50, 33.00, 371));
    System.err.println(determineRate(13015.00, 235.00, 60));
    System.err.println(determineRate(14713.16, 325.00, 48));
    System.err.println(determineRate(27110.25, 415.38, 72));
    System.err.println(determineRate(14819.92, 225.38, 72));
    System.err.println(determineRate(10000.00, 264.00, 48));
    System.err.println(determineRate(7900.00, 143.00, 59));
    System.err.println(determineRate(34970.94, 499.00, 84));
    System.err.println(determineRate(41110.61, 426.81, 120));
    System.err.println(determineRate(69627.21, 726.25, 120));
    System.err.println(determineRate(25609.00, 260.00, 120));
    System.err.println(determineRate(18100.00, 334.00, 59));
    System.err.println(determineRate(10559.00, 28.00, 92));
    System.err.println(determineRate(102000.00, 748.11, 181));
    System.err.println(determineRate(6614368.75, 0.01, 47));
    System.err.println(determineRate(12125.77, 174.00, 84));
    System.err.println(determineRate(25667.70, 385.33, 84));
    System.err.println(determineRate(1992.50, 75.00, 0));
    System.err.println(determineRate(1815.38, 75.00, 0));
    System.err.println(determineRate(3527.52, 25.00, 0));
    System.err.println(determineRate(13036.39, 262.00, 64));
    System.err.println(determineRate(22000.00, 190.00, 144));
}

private double determineRate(double loanAmount, double payment, int termInMonths) {
    //initial guess .05  5% rate.
    double rateGuess = 0.05;
    double calculatedPayment = 0.0;
    int wag = 19;
    int times = 0;
    do {
        times++;
        calculatedPayment = calculatePayment(loanAmount, rateGuess, termInMonths);

        if (payment < calculatedPayment) {
            //rate needs to go down by a percentage of the difference.
            double rateGuessPrior = rateGuess;
            rateGuess = Math.abs(rateGuess - Math.abs(((rateGuess * (1 - (Math.min(calculatedPayment, payment) / Math.max(calculatedPayment, payment)))) * Math.max(wag - times, 1))));
            if (rateGuessPrior < rateGuess) {
                rateGuess = rateGuessPrior - 0.01; // remove a percent. safety net.
            }
        } else {
            //rate needs to go up by a percentage of the difference.
            double rateGuessPrior = rateGuess;
            rateGuess = Math.abs(rateGuess + Math.abs(((rateGuess * (1 - (Math.min(calculatedPayment, payment) / Math.max(calculatedPayment, payment)))) * Math.max(wag - times, 1))));
            if (rateGuessPrior > rateGuess) {
                rateGuess = rateGuessPrior + 0.01; // remove a percent. safety net.
            }
        }
    } while (Math.max(payment, calculatedPayment) - Math.min(payment, calculatedPayment) > 0.05);
    System.err.println("It took: " + times + " times to complete.");
    System.err.println("for: " + loanAmount + ", " + payment + ", " + termInMonths);
    return Double.isNaN(rateGuess) ? 0.0 : BigDecimal.valueOf(rateGuess).multiply(BigDecimal.valueOf(100.0)).setScale(5, BigDecimal.ROUND_HALF_UP).doubleValue();
}

private double calculatePayment(double loanAmount, double rate, double termInMonths) {
    return (loanAmount * ((rate/12.0)*(Math.pow((1 + (rate/12.0)), termInMonths)))) / (Math.pow((1 + (rate/12.0)), termInMonths) - 1);
}
\$\endgroup\$
4
  • \$\begingroup\$ I would hate to be a maintainer of all those magic numbers \$\endgroup\$
    – syb0rg
    Jun 23 '16 at 18:07
  • \$\begingroup\$ @syb0rg -- yeah I'm wondering if a better approach might be down by 0.01 until I've gone to far then up by 0.001 until I've gone too far then down by 0.0001 etc...maybe I'll try that next. \$\endgroup\$
    – Uncle Iroh
    Jun 23 '16 at 18:12
  • \$\begingroup\$ I found very interesting your method. Please, what does mean "wag"? Thanks. \$\endgroup\$
    – user110014
    Jun 27 '16 at 18:14
  • \$\begingroup\$ Well wag typically stands for "Wild Ass Guess", in this case though I'm using it mostly as a way to speed up the closing of the gap of the initial amount I'm off. Assuming the initial guess of 5% is off by a lot, I found that I could reduce the number of guesses by multiplying the initial amount it adjusts by 19, then 18, and so on down to 1 where it stays forever. That way if does the calculation and realizes it needs to adjust up, it adjusts up by a LOT the first time, then less the second time etc. So it's sort of a ramp off process. In my case it helps with the data sets I tried. \$\endgroup\$
    – Uncle Iroh
    Jun 27 '16 at 19:36
3
\$\begingroup\$

Unit testing

public void testItAll() { ... }

Kind of a good start...

    System.err.println(determineRate(226.34, 25.00, 12));

Eh, what? One, why are you printing out on the standard error stream? Two, how do you complete the so-called 'testing'? By manually checking the output matches?

This is where unit testing comes in! A proper unit test should let a developer:

  1. Specify some test input.
  2. Process the input to some output to be recorded.
  3. Assert that the recorded output matches an expected result.

There are a handful of good Java unit testing frameworks, and I will use TestNG as an example below.

You start off by having annotating a test method so that the framework recognizes that it needs to run that test (this is dependent on your unit testing framework), and an underlying method that calls your method to test:

@Test
public void testOne() {
    doTest(226.34, 25.00, 12, 55.50992);
}

public void testLoanRate(double loanAmount, double payment, int termInMonths,
                            double expectedRate) {
    // ...
}

As you can see, its method signature is very similar to your method, because you need the desired inputs. The extra parameter, expectedRate, lets you assert that the calculation is correct. The body of the method can be something like:

public void testLoanRate(double loanAmount, double payment, int termInMonths,
                            double expectedRate) {
    // assuming determineRate() can be made static
    double result = determineRate(loanAmount, payment, termInMonths);
    // following static method is provided by TestNG
    // the third argument controls the absolute tolerable difference,
    // an arbitrary small amount is chosen as an example
    Assert.assertEquals(result, expectedRate, 0.0001d);
    // optionally print inputs and output when successful
    System.out.printf("Amount: %f, Payment: %f, Term in months: %d, Result: %f%n",
                        loanAmount, payment, termInMonths, result);
}

Testing a number of inputs and expected results is relatively easy with TestNG's parameterized testing feature. You need a @DataProvider, and link it up with the @Test method on the testLoanRate() method now:

@DataProvider(name = "test-cases")
public Iterator<Object[]> getTestCases() {
    return Arrays.asList(
        new Object[] { 226.34, 25.00, 12, 55.50992 },
        new Object[] { 5800.00, 29.00, 31, 0 }, // ???
        new Object[] { 4000.00, 25.00, 460, 6.97672 },
        /* ... */
    ).iterator();
}

@Test(dataProvider = "test-cases")
public void testLoanRate(double loanAmount, double payment, int termInMonths,
                            double expectedRate) {
    double result = determineRate(loanAmount, payment, termInMonths);
    Assert.assertEquals(result, expectedRate, 0.0001d);
    System.out.printf("Amount: %f, Payment: %f, Term in months: %d, Result: %f%n",
                        loanAmount, payment, termInMonths, result);
}

When you run this unit test, TestNG will iterate through all the test cases to assert the results. This eliminates the need to manually check every calculation.

Magic numbers and math

private double calculatePayment(double loanAmount, double rate, double termInMonths) {
    return (loanAmount * ((rate/12.0)*(Math.pow((1 + (rate/12.0)), termInMonths)))) 
                / (Math.pow((1 + (rate/12.0)), termInMonths) - 1);
}

Using just one example, 12.0 seems to be used a lot here. You should consider putting your magic numbers into an easily identifiable constant so that they can be reused in a consistent and readable manner.

Math.max(payment, calculatedPayment) - Math.min(payment, calculatedPayment) > 0.05

Another way of expressing this can be just:

Math.abs(payment - calculatedPayment) > 0.05
\$\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.