Jamie West Jamie West - 1 month ago 14
Java Question

How to fix leaking accessor methods?

so I am having trouble figuring out why my test in JUnit is failing. I have a

Bill
class, a
Money
class, and a
Date
class. A new
Bill
object is being created in the test and the line

assertTrue( myBill.getAmount().getCents() == 0);


is failing. So I am aware of where it is happening but I'm not exactly sure how to fix it. I have tried changing my mutator methods to things like

return new Date(dueDate);


instead of just

return dueDate;


but it is still failing in JUnit. Please help!

Test code:

@Test
public void testBillConstructorPrivacyLeak()
{
Date date1 = new Date( 1, 1, 2020);
Money money1 = new Money( 10);
Bill myBill = new Bill( money1, date1, "sam");

date1.setYear( 2021);
money1.setMoney( 5, 10);

//Now get values and make sure they have not changed
assertTrue( myBill.getAmount().getCents() == 0);
assertTrue( myBill.getDueDate().getYear() == 2020);
}


My classes:

public class Bill
{
private Money amount;
private Date dueDate;
private Date paidDate;
private String originator;

//paidDate set to null
public Bill (Money amount, Date dueDate, String originator) {
this.amount = amount;
this.dueDate = dueDate;
this.originator = originator;
paidDate = null;
}

//copy constructor
public Bill (Bill toCopy) {
this.amount = toCopy.amount;
this.dueDate = toCopy.dueDate;
this.paidDate = toCopy.paidDate;
this.originator = toCopy.originator;
}

public Money getAmount () {
return new Money(amount);
}

public Date getDueDate () {
return new Date(dueDate);
}

public String getOriginator () {
return originator;
}

//returns true if bill is paid, else false
public boolean isPaid () {
return (paidDate != null);
}

//if datePaid is after the dueDate, the call does not update anything and returns false.
//Else updates the paidDate and returns true
//If already paid, we will attempt to change the paid date.
public boolean setPaid (Date datePaid) {
if (datePaid.isAfter(dueDate)) {
return false;
}
else {
paidDate = new Date(datePaid);
return true;
}
}

//Resets the due date – If the bill is already paid, this call fails and returns false.
//Else it resets the due date and returns true.
public boolean setDueDate (Date newDueDate) {
if (isPaid()) {
return false;
}
else {
dueDate = new Date(newDueDate);
return true;
}
}

//Change the amount owed.
//If already paid returns false and does not change the amount owed else changes
//the amount and returns true.
public boolean setAmount (Money amount) {
if (isPaid()) {
return false;
}
else {
amount = new Money(amount);
return true;
}
}

public void setOriginator (String originator) {
this.originator = originator;
}

//Build a string that reports the amount, when due, to whom, if paid, and if paid
//the date paid
public String toString () {
return "Amount: " + amount + " Due date: " + dueDate + " To: " + "originator" + " Paid?" + isPaid() + "Paid date: " + paidDate;
}

//Equality is defined as each field having the same value.
public boolean equals (Object toCompare) {
if (toCompare instanceof Bill) {
Bill that = (Bill) toCompare;
return this.amount.equals(that.amount) &&
this.dueDate.equals(that.dueDate) &&
this.paidDate.equals(that.paidDate) &&
this.originator.equals(that.originator);
}
return false;
}


}

public class Money
{
private int dollars;
private int cents;

//Constructor which sets the dollar amount, and sets cents to 0
//If the user enters in an amount LT 0, you will throw an IllegalArgumentException
public Money (int dol) {
if (dol < 0) {
throw new IllegalArgumentException ("Must be greater than 0.");
}
this.dollars = dol;
cents = 0;
}

//Constructor which initialized dollars and cents.
//If the user enters in an amount LT 0, you will throw an IllegalArgumentException
public Money (int dol, int cent) {
if (dol < 0 || cent < 0) {
throw new IllegalArgumentException ("Must be greater than 0.");
}
this.dollars = dol;
this.dollars += cent / 100;
this.cents = cent % 100;
}

//Copy constructor
public Money (Money other) {
this.dollars = other.dollars;
this.cents = other.cents;
}

public int getDollars () {
return dollars;
}

public int getCents () {
return cents;
}

//If the user enters in an amount LT 0, you will throw an IllegalArgumentException
public void setMoney (int dollars, int cents) {
if (dollars < 0 || cents < 0) {
throw new IllegalArgumentException ("Must be greater than 0.");
}
this.dollars = dollars;
this.dollars += cents / 100;
this.cents = cents % 100;
}

//Gets the money amount as a double
//For example it might return 5.75
public double getMoney () {
return dollars + (cents / 100.0);
}

//If the user enters in an amount LT 0, you will throw an IllegalArgumentException4
public void add (int dollars) {
if (dollars < 0) {
throw new IllegalArgumentException ("Must be greater than 0.");
}
this.dollars += dollars;
}

//If the user enters in an amount LT 0, you will throw an IllegalArgumentException
public void add (int dollars, int cents) {
if (dollars < 0 || cents < 0) {
throw new IllegalArgumentException ("Must be greater than 0.");
}
this.dollars += dollars;
this.cents += cents;
this.dollars += this.cents / 100;
this.cents = this.cents % 100;
}

//Adds the amounts in other to our money object – reducing cents appropriately.
public void add (Money other) {
this.dollars += other.dollars;
this.cents += other.cents;
this.dollars += this.cents / 100;
this.cents = this.cents % 100;
}

//Two money objects are the same if they have the same value for dollars and cents.
public boolean equals (Object o) {
if( o instanceof Money) {
return this.dollars == ((Money)o).dollars && this.cents == ((Money)o).cents;
}
return false;
}

//Prints out the amount as a string IE “$3.75” or “$4.00” Note the number of digits displayed for cents.
//Again for testing and grading purposes use this EXACT output format
public String toString () {
String c = String.format("%.02d",cents);
return "$" + dollars + "." + c;
}


}

Answer

Your problem results from the fact that in your constructor for Bill you store references to the Money and Date objects. Then, when you modify those objects in the test case you are modifying the same objects.

If you don't want that behavior you have to make a deep copy of the Money and Date objects in the Bill constructor, i.e.:

public Bill (Money amount, Date dueDate, String originator) {
    this.amount = new Money(amount);
    this.dueDate = new Date(dueDate);
    this.originator = originator;
    paidDate = null;
}

You don't have to do this for originator because Strings are immutable.