Search code examples
javasonarqubegetter-setter

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


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?


Solution

  • 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