Java Class Constructor Fix to Sonar

Question

I need to correct some differences in sonar. I have these builders, getters and setters:

public EmpregadoContrato() {
    // construtor padrão
}

public EmpregadoContrato(Integer id) {
    this.id = id;
}


public Contrato getContrato() {
    return contrato;
}

public void setContrato(Contrato contrato) {
    this.contrato = contrato;
}

How do I fix this Constructor that is error in Sonar:

public EmpregadoContrato(Integer id) {
    this.id = id;
}

Error:

"contract" is marked "javax.validation.constraints.Notnull" but is not initialized in this constructor.

Answer 1

If contrato was marked with the annotation @NotNull, then it means that this field cannot be null.

But in your builders, or you just arrow the id, or do not arrow any of the fields (in the default constructor). How contrato is not set in any of them, so it will be null - which is just what it cannot be, as it was marked as @NotNull.

Therefore, you could remove the two builders (the default and what gets the id) and leave only this:

public EmpregadoContrato(Integer id, Contrato contrato) {
    this.id = id;
    this.contrato = contrato;
}

Reflect: if contrato can’t be null, makes sense to have builders who create a EmpregadoContrato with contrato null? Both builders you have create this situation (contrato null), then either these constructors must be removed, or maybe the field can be null.

If the field can be null, remove the annotation @NotNull.

If the field really cannot be null, then it makes no sense to have constructors that do not set any value to this field.

Of course that doesn’t stop someone from trying to do new EmpregadoContrato(1, null), but in this case just include a validation in the constructor. For example:

public EmpregadoContrato(Integer id, Contrato contrato) {
    this.id = id;
    if (contrato == null) {
        throw new IllegalArgumentException("contrato não pode ser nulo");
    }
    this.contrato = contrato;
}

Another option is to use Objects.requireNonNull, the difference is that this method launches a NullPointerException if the value is null:

public EmpregadoContrato(Integer id, Contrato contrato) {
    this.id = id;
    // se for nulo, lança NullPointerException, caso contrário, retorna o próprio valor
    this.contrato = Objects.requireNonNull(contrato, "contrato não pode ser nulo");
}

The same goes for the id. Is this field required? If it is, remove the default constructor, otherwise you will create an instance without id. Only create constructors that make sense (suggested reading: "What good is a builder?").

---

Of course, depending on the case, another option would be:

public EmpregadoContrato(Integer id) {
    this.id = id;
    this.contrato = new Contrato();
}

Here I created some contract, just so the field is not null. But you have to assess whether this makes sense: it’s okay to have an "empty" contract (I’m guessing the class Contrato has fields that need to have some value to represent a valid contract)? Until because, in the class Contrato are worth the same questions: does it make sense to create a contract with no set value? Or it is better to only have constructors who receive the information needed to create a valid instance?

You could even have a different implementation for an "empty contract", using the Null Object standard, but should evaluate according to the context if it is worth it - maybe it is better to only have the builder who requires to pass some valid contract.