Java- How can I fix my methods so that there are no privacy leaks?

Question

I have a constructor and accesor method which are supposed to not have any privacy leaks but when I run my JUnit tests I am not getting my intended output.

This is my constructor

public Date(int month, int day, int year){
    if(day < 1 || day > 31){
        System.out.println("invalid day: " + day);
        System.exit(0);
    }else if(month < 1 || month > 12){
        System.out.println("invalid month: " + month);
        System.exit(0);
    }else if(year < 2014 || year > 2024){
        System.out.println("invalid year: " + year);
        System.exit(0);
    }
    this.month = month;
    this.day = day;
    this.year = year;
}

And this is my accesor method

public void setDay(int day) {
    if (day >= 1 || day <= 31)
       this.day = day;
    }

The JUnit test I ran was this

@Test 
public void OrderDatePrivacyLeaks()
{
    // make sure date was copied when set
    Date d = new Date(6,12,2017);
    Order b = new Order(new Money(2,33), d, "ACME Company", "widget");
    d.setDay(10);
    Date billDate = b.getOrderDate();
    assertEquals(12, billDate.getDay());
}

And the setDay should not be able to change the values of b so the expected output is 12 yet I am getting 10. Could someone tell me what the issue is?

You are referencing an object when you put date inside order you are not copying date you are just referencing it so the moment you change the original object every thing that its referencing it changes — Anon, Nov 28 '22 at 04:27

Boann · Answer 1 · 2022-11-28T04:58:14.103

"Privacy leak" is odd terminology, but basically you have created a mutable Date class, but then the Order class uses it without defensively copying and protecting access to its instance of it.

Your options are:

(A) Make sure the Order class defensively protects its instance of the Date class so that outside code cannot get access to it. That means keep the date object on a private field, and in the constructor, and any setters or getters, make sure you duplicate the object; do not just return or assign it, because then it will leak. You haven't shown the Order class, but something like:
```
  class Order {
      private Date orderDate;

      Order(Date orderDate) {
          this.orderDate = new Date(orderDate);
          ...
      }

      Date getOrderDate() {
          return new Date(orderDate);
      }
  }
```
This assumes that Date has a second constructor which will make a copy of an existing Date object, or you could give it a clone() method if you prefer that style.
(B) Make this Date class of yours immutable. That means delete Date.setDay, make the class final, and make its fields final. Then you don't need to worry about copying it. Small objects representing values like dates are good candidates for being made immutable:
```
  final class Date {
      final int year, month, day;

      ...
  }
```

You'll probably need to choose a fix for your Money class too: defensive copying, or make it immutable.

Also, don't kill forcibly kill the application for a bad constructor argument:

if(day < 1 || day > 31){
    System.out.println("invalid day: " + day);
    System.exit(0);

Throw an exception, so that callers have flexibility in how they handle the error:

if (day < 1 || day > 31) throw new IllegalArgumentException("invalid day: " + day);

Java- How can I fix my methods so that there are no privacy leaks?

1 Answers1