2

Sonar says that mutable members should not be stored or returned directly

Mutable objects are those whose state can be changed. For instance, an array is mutable, but a String is not. Mutable class members should never be returned to a caller or accepted and stored directly. Doing so leaves you vulnerable to unexpected changes in your class state.

Instead, a copy of the mutable object should be made, and that copy should be stored or returned.

This rule checks that arrays, collections and Dates are not stored or returned directly.

I'm trying to store one date. To solve the sonar alert, I created two helper methods to return a copy of my original date.

Example A:

public static Date clone(Date originalDate) {
    if(date != null) {
        return new Date(originalDate.getTime());
    }
    return null;
}

Example B:

public static Date clone(Date originalDate) {
    if (date != null) {
        return  (Date)originalDate.clone();
    }
    return null;    
}

If I use the constructor way (example A), I loose the possible extra information that contains my original date.

Should I copy my originalDate with the constructor (example A) or with clone (example B)?

Jesus Zavarce
  • 1,729
  • 1
  • 17
  • 27
  • 2
    It doesn't really matter which one you choose. I would write: `return originalDate != null ? new Date(originalDate.getTime()) : null;` – Jesper Jun 09 '16 at 14:49
  • 1
    If you want to make it immutable, you could store `date.getTime()` as a `final long`. Then just rehydrate it to a `new Date(fieldName)` when you need it. If you want slightly stronger typing, create a trivial wrapper class for the long with `toDate()`/`fromDate()` methods. – Andy Turner Jun 09 '16 at 14:56
  • If I were you I wouldn't mess with `java.util.Date`. If you have a chance to use JDK 8 I would propose you to use Instant, otherwise you could simply store epoch milliseconds or Joda Time. – ppopoff Jun 10 '16 at 16:55

1 Answers1

3

Should I copy my originalDate with the constructor (example A) or with clone (example B)?

Choose the way that is more readable. i.e. example A.

However Ternary Operator has got a use here.

And you might want to look here: Clone() vs Copy constructor- which is recommended in java

Community
  • 1
  • 1
Azodious
  • 13,752
  • 1
  • 36
  • 71
  • Even if it is often assumed that Date is just a wrapper around a long value, that is actually not true. `new Date(oldDate.getTime())` is not just a simple 'copy constructor call', `getTime()` may require some calculations to return a result and the new object is not necessarily technically equal (though semantically) to the old Date object. – jarnbjo Jun 09 '16 at 15:54