Java Confusing Ternary Sonar Violation Issue

Viewed 407

Sonar reported Confusing Ternary violation against the below code:

package com.example.dto;

import java.util.Date;

public class ShiftTemplateUserDTO {
  private Date breakStartTime;
  private Date breakEndTime;
  
  public Date getBreakStartTime() {
    return breakStartTime != null ? new Date(breakStartTime.getTime()) : null;
  }

  public void setBreakStartTime(Date breakStartTime) {
    this.breakStartTime = breakStartTime != null ? new Date(breakStartTime.getTime()) : null;
  }

  public Date getBreakEndTime() {
    return breakEndTime != null ? new Date(breakEndTime.getTime()) : null;
  }

  public void setBreakEndTime(Date breakEndTime) {
    this.breakEndTime = breakEndTime != null ? new Date(breakEndTime.getTime()) : null;
  }
}

I tried updating the code to add if/else condition but still Sonar is complaining about the same rule, what am I doing wrong?

  public Date getBreakStartTime() {
    if (breakStartTime != null) {
      return new Date(breakStartTime.getTime());
    } else {
      return null;
    }
  }

  public void setBreakStartTime(Date breakStartTime) {
    if (breakStartTime != null) {
      this.breakStartTime = new Date(breakStartTime.getTime());
    } else {
      this.breakStartTime = null;
    }
  }
3 Answers

Do not use negated conditions in ternary operators. Try replacing this:

return breakStartTime != null ? new Date(breakStartTime.getTime()) : null;

with this:

return breakStartTime == null ? null : new Date(breakStartTime.getTime());

The same applies to your if-else logic. Replace this:

if (breakStartTime != null) {
  return new Date(breakStartTime.getTime());
} else {
  return null;
}

with this:

if (breakStartTime == null) {
  return null;
} else {
  return new Date(breakStartTime.getTime());
}

Sonar doesn't like "un-equal" comparisons. It wants you to write foo == null ? null : ....

You also might consider that instead of doing this:

public Date getBreakStartTime() {
    return breakStartTime != null ? new Date(breakStartTime.getTime()) : null;
}

You just do this:

public Date getBreakStartTime() {
    return breakStartTime;
}

In your current code, if the Date object is not null, you are constructing a new Date object with the same time value as the original Date value. That results in effectively the same object. You might be concerned about immutability. Is that really a concern for you?

Related