Nullcheck elegante

Programação
#1

Boa tarde pessoal.

Estou fazendo refactoring num sistema BEM antigo e me deparei com coisas desse tipo:

if(this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument().	getFormaat().contains("pdf")) {
                //do stuff
                }

Inclusive em alguns casos isso gera NullPointerExcpetion, minha pergunta é: Qual é a melhor maneira de evitar uma Exception nesse caso ? Fazer um nullCheck em cada objeto funciona, mas o codigo fica péssimo, ficaria tipo isso:

if(this.getZaakdocument() != null){
				if(this.getZaakdocument().getIsGerelateerdAan() != null){
					if(this.getZaakdocument().getIsGerelateerdAan().getDocument() != null){
						if(this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument() != null){
							if(this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument() != null){
								if(this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument().getFormaat().contains("pdf") ){
									//do stuff
								}
							}
						}
					}
				}
			}

Mesmo se atribuir os gets para algum objeto, ainda ficaria bem feio.
Qual é a maneira mais elegante de resolver isso ?

#2

Acredito que a melhor forma é utilizar Try Catch.

#3

Sim está uma zona.

Depende do objetivo da funcionalidade. Esse código está tão confuso que não dá pra entender nada. Se essa informação vem do banco, faria uma query já trazendo o resultado final. Mas é melhor você explicar o objetivo.

#4

Vejo isso diariamente e isso só reforça minha opinião de que os alemães são muito bons no ramo automobilístico mas péssimos quando o assunto é software.

Códigos desse tipo só deveriam ser escritos quando se tem a certeza de que em nenhum momento das chamadas encadeadas ocorrerá NullPointerException.
E essa certeza só existe quando se implementa muito bem uma uma interface fluente ou o padrão Builder, o que não é o caso nesse tipo de código legado…

Evite escrever código assim:

if (this.getZaakdocument() != null){
    if (this.getZaakdocument().getIsGerelateerdAan() != null){
        if (this.getZaakdocument().getIsGerelateerdAan().getDocument() != null){
            if (this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument() != null){
                if (this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument() != null){
                    if (this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument().getFormaat().contains("pdf") ){
                    //do stuff
                    }
                }
            }
        }
    }
}

Percebeu quanto código repetido?

chamou 6 vezes o getZaakdocument(),
5 vezes o getIsGerelateerdAan(),
4 vezes o getDocument(),
3 vezes o getIsEnkelvoudigDocument() e
2 vezes o getEnkelvoudigDocument().

Ao invés disso utilize variáveis locais, é mais elegante e a execução é mais rápida.

Eu também gosto de separar esse tipo de lógica gigante em métodos, como por exemplo:

private boolean isFormat(String format) {
    Zaakdocument zaakdocument = this.getZaakdocument();
    if (zaakdocument == null) {
        return false;
    }
    IsGerelateerdAan isGerelateerdAan = zaakdocument.getIsGerelateerdAan();
    if (isGerelateerdAan == null) {
        return false;
    }
    Document document = isGerelateerdAan.getDocument();
    if (document == null) {
        return false;
    }
    IsEnkelvoudigDocument isEnkelvoudigDocument = document.getIsEnkelvoudigDocument();
    if (isEnkelvoudigDocument == null) {
        return false;
    }
    EnkelvoudigDocument enkelvoudigDocument = isEnkelvoudigDocument.getEnkelvoudigDocument();
    if (enkelvoudigDocument == null) {
        return false;
    }
    Formaat formaat = enkelvoudigDocument.getFormaat();
    if (formaat == null) {
        return false;
    }
    return formaat.contains(format);
}

Aí ao invés de escrever:

if (this.getZaakdocument().getIsGerelateerdAan().getDocument().getIsEnkelvoudigDocument().getEnkelvoudigDocument().getFormaat().contains("pdf")) {
    //do stuff
}

É só escrever:

if (isFormat("pdf")) {
    //do stuff
}
1 curtida
#5

Outra forma seria usando o Optional:

    Optional<Format> resultado = Optional
        .ofNullable(getZaakdocument())
        .flatMap(o -> Optional.ofNullable(o.getIsGerelateerdAan()))
        .flatMap(o -> Optional.ofNullable(o.getDocument()))
        .flatMap(o -> Optional.ofNullable(o.getIsEnkelvoudigDocument()))
        .flatMap(o -> Optional.ofNullable(o.getFormaat()))
        .filter(o -> o.contains("pdf"));
    
    if (resultado.isPresent()) {
        System.out.println("Executa Tarefa");
    } else {
        System.out.println("Nao Executa Tarefa");
    }
2 curtidas
#6

Exatamente, é uma ótima alternativa.
Tem que ver se o sistema legado em que nosso colega trabalha permite o uso do Java 8.
Eu por exemplo trabalho com um software alemão que só pode ser implementado com Java 6. :frowning:

#7

Grande sugestao.
Aliás, usando o map ao invés de flatMap você nem precisa converter o valor para optional novamente, simplificando ainda mais o código!

#8

Percebeu quanto código repetido?
Ao invés disso utilize variáveis locais, é mais elegante e a execução é mais rápida.

Sim! Foi isso que quis dizer com “Mesmo se atribuir os gets para algum objeto, ainda ficaria bem feio.”

Vejo isso diariamente e isso só reforça minha opinião de que os alemães são muito bons no ramo automobilístico mas péssimos quando o assunto é software.

Na verdade esses são Holandeses… mas é tudo meio que a mesma coisa! :joy:

Acabei escrevendo um método parecido com esse seu “isFormat”, tá limpo, mas fica bem grande comparado com a gambiarra que só tinha uma linha… pelo menos agora não dá NP!

1 curtida
#9

Quem me dera! Tudo aqui é Java 7 :expressionless:

#10

Imaginei, europeus costumam ser resistentes à mudanças.

#11

Neste caso eu criaria um método na classe ou um “util” para acessar diretamente o format, algo mais ou menos:

class Classe {
  public Zaakdocument getZaakdocument() { ... }
  public IsGerelateerdAan getIsGerelateerdAan() { return getZaakdocument()==null ? null : getZaakdocument().getIsGerelateerdAan(); }
  public Document getDocument() { return getIsGerelateerdAan()==null ? null : getIsGerelateerdAan().getDocument(); }
  public IsEnkelvoudigDocument getIsEnkelvoudigDocument() { return getDocument()==null ? null : getDocument().getIsEnkelvoudigDocument(); }
  public EnkelvoudigDocument getEnkelvoudigDocument() { return getIsEnkelvoudigDocument()==null ? null : getIsEnkelvoudigDocument().getEnkelvoudigDocument(); }
  public Formaat getFormaat() { return getEnkelvoudigDocument()==null ? null : getEnkelvoudigDocument().getFormaat(); }
}

A sugestão do @staroski também é boa.