0

I'm working on a program that processes bank transactions. My withdraw method subtracts the amount from the balance and if there aren't enough funds for the withdrawal, an error message is outputted to the screen. The error message is the case I am having trouble testing.

public void withdraw(double amt) {
    double bal = getBalance();
    if(bal >= amt) {
        super.setBalance(bal - amt);
        if(bal < minBalance) {
            super.setBalance(bal - amt - maintFee);
        }   
    } else {
        System.out.println("Error: Not enough funds for withdraw");
    }       
}

These are my JUnit tests for this method currently. I just need help on testWithdrawThree(). Thank you!

@Test
void testWithdrawOne() {
    Savings s = new Savings(500.00, 30.00, "111", "Andrew Green", 1000.00);
    s.withdraw(200);
    assertEquals(800.00, s.getBalance());
}

@Test
void testWithdrawTwo() {
    Savings s = new Savings(500.00, 30.00, "111", "Andrew Green", 400.00);
    s.withdraw(200.00);
    assertEquals(170.00, s.getBalance());
}

@Test
void testWithdrawThree() {
    Savings s = new Savings(500.00, 30.00, "111", "Andrew Green", 400.00);
    s.withdraw(600.00);
    //Need to check that program will output "Error: Not enough funds for withdrawal"

}
Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
MikeySg
  • 19
  • 1
  • How is the rest of your program supposed to know that something went wrong? This is exactly what exceptions are for (in this case, `IllegalStateException` or your own subclass). – chrylis -cautiouslyoptimistic- Nov 28 '18 at 23:32
  • What about replacing out stream by a `Logger`? This way, it will be simpler to mock and events can later go to a file without code modification if needed? – Loïc Le Doyen Nov 29 '18 at 16:31

2 Answers2

2

Best approach in my eyes is doing a refactoring as suggested by Yoni already. Alternatively to changing the signature of the method (maybe it's not possible because it's defined in an interface), your original method can call another method that you pass the PrintStream to be used:

public void withdraw(double amt) {
    performWithdraw(amt, System.out);
}

void performWithdraw(double amt, PrintStream errorStream) {
    double bal = getBalance();
    if(bal >= amt) {
        super.setBalance(bal - amt);
        if(bal < minBalance) {
            super.setBalance(bal - amt - maintFee);
        }   
    } else {
        errorStream.println("Error: Not enough funds for withdraw");
    }       
}

Your test class (residing in the same package and therfor able to access performWithdraw) would look something like this:

@Test
void testInvalidWithdraw() {
    ByteArrayOutputStream baos = new ByteArrayOutputStream();
    PrintStream ps = new PrintStream(baos, true, "utf8");
    Savings s = new Savings(500.00, 30.00, "111", "Andrew Green", 400.00);
    s.performWithdraw(600.00, ps);
    assertEquals(400d, s.getBalance());
    assertEquals("check error message", "Error: Not enough funds for withdraw", baos.toString("utf8"));
}

BTW: You should test that the balance is unchanged (I added a corresponding assertEquals in my example) and you should check edge cases as well, i.e. check that you get the error message when withdrawing 400.01. Edge cases should also be checked for the cases where mainenance fees are charged or aren't.

BTW2: Using double for monetary amounts is a Bad Thing[TM]. For learning JUnit it's OK but for Real Applications[TM] you should e.g. use BigDecimal instead.

Lothar
  • 5,323
  • 1
  • 11
  • 27
1

There are a few ways that you can go about this:

  1. You can take ownership of the System.out stream, as explained in this answer and Andrea mentioned in the comment. I don't think this is a very good route to go because it means that you won't be able to run your tests in parallel since System.out is a shared resource across the JVM.
  2. You can refactor your code - maybe your method should return an error if the withdrawal is not successful? You are basically swallowing the error rather than reporting it.
  3. You can test that the balance did not change after the invocation of the method without sufficient funds. I think this is actually the interesting behavior of the system that you want to test, isn't it? Testing the error message that's printed seems superficial. Maybe it will give you coverage for that line of code but is it really the meaning that you want to capture in your test? Is anyone really going to look at the output stream in production and look for this message? This is again another argument to refactor your code ...

Also, a side note: it would go a long way to name your test methods with meaningful names, e.g. testWithdrawWithoutFunds, instead of the generic names testOne, testTwo, etc.

Yoni
  • 10,171
  • 9
  • 55
  • 72