Nullcheck elegante

16 de março de 2018 10 respostas

andre_a_s 16 de março de 2018

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 ?

10 Respostas

Vinicius_Salles 16 de mar. de 2018

Acredito que a melhor forma é utilizar Try Catch.

javaflex 16 de mar. de 2018

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.

staroski 16 de mar. de 2018 1 like

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
}

diego12 16 de mar. de 2018 2 likes

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");
    }

staroski 17 de mar. de 2018

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.

AbelBueno 17 de mar. de 2018

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!

andre_a_s 19 de mar. de 2018 1 like

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!

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!

andre_a_s 19 de mar. de 2018

Quem me dera! Tudo aqui é Java 7

staroski 19 de mar. de 2018

Imaginei, europeus costumam ser resistentes à mudanças.

diego12 19 de mar. de 2018

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.

Criado 16 de março de 2018

Ultima resposta 19 de mar. de 2018

Respostas 10

Participantes 6

10 Respostas

Topicos relacionados