Solve Sonar issue in simple getter and setter methods with Date or List

Question

I write this getter/setter to list from Eclipse source menu:

public Date getDate() {
    return date;
}

public void setDate(Date date) {
    this.date = date;
}

And Sonar reporting two issues:

Return a copy of "date" & Store a copy of "date"

with the explanation

"Mutable members should not be stored or returned directly"

and a example code:

public String [] getStrings() {
    return strings.clone();}

public void setStrings(String [] strings) {
    this.strings = strings.clone();}

I think if my Date is null, it will throw a NullPointerException. Then I've changed my code to:

public Date getDate() {
    if (this.date != null) {
        return new Date(this.date.getTime());
    } else {
        return null;
    }
}

public void setDate(Date date) {
    if (date != null) {
        this.date = new Date(date.getTime());
    } else {
        this.date = null;
    }
}

And now marks other issue:

"Assigning an Object to null is a code smell. Consider refactoring".

I've searched in internet and set or return a new array is not a solution for me, I want to preserve my list to null if the setter param is null to overwrite an existing previous list.

I've the same problem for List, and I want to return/preserve null instead of a new ArrayList for an empty List. And in this case, the setter marks one more issue:

"Return an empty collection instead of null.".

What is the solution for this issue?

Answer 1

If you are in Java 8 and do not want to handle empty date, then maybe usage of Optional would help you.

Edit: Example of your "POJO" class

public class Toto {

    public Optional<Date> myDate;

    public Optional<Date> getMyDate() {
        return this.myDate;
    }

    public void setMyDate(final Date myDate) {
        this.myDate = Optional.ofNullable(myDate);
    }

}

Example of code usage:

Toto toto = new Toto();
toto.setMyDate(null);
System.out.println("Value is null ? " + toto.getMyDate().isPresent());
System.out.println("Value: " + toto.getMyDate().orElse(new Date()));

Try to change the toto.setMyDate(...) with concrete date value to see what happen.

If you don't know what is Optional or how to use it, you can find plenty of examples.

BUT : This is only a way to solve your violation issue and i totally agree with Brad's remark, Optional are not intent to be used as a type, but more like a contract for potential empty / null returns. In general, you should not correct your code in a bad way just to fix a violation, if the violation is not correct. And in your case i think you should just ignore the violation (as most of Sonar's one unfortunatly)

If you really want to use Java 8 and Optional in your code, then you POJO class would be like this (usage of Optional as a contrat on the getter only)

public class Toto {


    public Date myDate;

    public Optional<Date> getMyDate() {
        return Optional.ofNullable(this.myDate);
    }

    public void setMyDate(final Date myDate) {
        this.myDate = myDate;
    }

}

This way,

• You bean stay serializable (Optional is not)
• You still enable your "client" code to have the choice on how to behave to empty / null value of your property
• Configure your Sonar violation as a false positive as it is what you want instead of changing your code

Answer 2

Generally, while using static analysis tools to verify the code is valuable, you should not blindly fix every warnings which popups on you. You need to analyze the issue which is triggered and check if it really applies in your context.

Now to address the issues you are mentioning

Return a copy of "date" & Store a copy of "date"

This seems to be valid one. It is good practice to be defensive and not expose mutable state via getters/setters. So creating a defensive copy in getter/setter should be done. This can be done the way you did it, or by using new Java Time API, which provides immutable objects.

Assigning an Object to null is a code smell. Consider refactoring

IMO dubious one. The issue is raised by PMD plugin (which is the tool analyzing the code, SonarQube is displaying the report). Issue is raised by this rule http://pmd.sourceforge.net/pmd-4.3.0/rules/controversial.html#NullAssignment , as you can see it is in controversial category. I don't think there is anything wrong with your code, and proper action might be to ignore this warning and mark the issue as "won't fix". You can also configure your SonarQube to not use this particular rule in your Quality Profile setting.

Return an empty collection instead of null.

You did not provide the code which is triggering it, but this seems to be a valid piece of advice. It is generally better to return empty collections rather than nulls.