4

O SonarLint para Eclipse, acusa o erro:

Refactor this method to reduce its Cognitive Complexity from 64 to the 15 allowed.

Reescreva esse metodo para reduzir sua complexidade cognitiva de 64 para 15 permitidos

Ou seja, meu método tem muitos ifs e elses, muitos pontos de decisão e eu preciso diminuir isso.

Em um método de com regras de negócio isso até que é "fácil", reescrevo o método separando em vários outros métodos pequenos e "JUnitizáveis".

O problema é o método equals(), possuo alguns objetos com vários campos, e o método equals() precisa verificar a similaridade de todos esses campos, cheque de null, de equals e == para primitivos.

Esse meu equals() já é um método com contexto simples, ele só faz uma tarefa, já é testável por JUnit, porém possui diversos pontos de decisão dentro.

O que fazer nesse caso? Criar vários métodos, um para testar null, um para testar equals(), um para testar primitivos e juntar todos no equals() principal? Tem alguma abordagem melhor?

OBS: Sei que posso juntar vários ifs em uma única linha, mas isso atrapalha muito a visualização por um ser humano e estou tentando evitar essa prática.

Exemplo, para essa classe o SonarLint está acusando que ela possui complexidade de 34:

package testes;

import java.math.BigDecimal;

public class EntidadeGrande {

    private int codUser;
    private String descUser;
    private long idUser;
    private BigDecimal valueX;
    private int scoreUser;
    private int scoreStore;
    private int cdLogo;
    private String nmMother;
    private String sgEstado;
    private String dsBandeira;
    private String codCupom;

    /* UM MONTE DE GET E SET AQUI, HASHCODE TAMBÉM */


    @Override
    public boolean equals(Object o) {
        if (o == null || o.getClass() != getClass()) {
            return false;
        }

        final EntidadeGrande e = (EntidadeGrande) o;
        if (getCodUser() != e.getCodUser() 
                || getIdUser() != e.getIdUser()
                || getScoreUser() != e.getScoreUser()
                || getScoreStore() != e.getScoreStore()
                || getCdLogo() != e.getCdLogo()) {
            return false;
        }

        if (getValueX() == null && e.getValueX() != null
                || getValueX() != null && e.getValueX() == null) {
            return false;
        } 

        if (!getValueX().equals(e.getValueX())) {
            return false;
        }

        if (getDescUser() != null && e.getDescUser() == null
                || getDescUser() == null && e.getDescUser() != null) {
            return false;
        }

        if (getNmMother() != null && e.getNmMother() == null
                || getNmMother() == null && e.getNmMother() != null) {
            return false;
        }

        if (getSgEstado() != null && e.getSgEstado() == null
                || getSgEstado() == null && e.getSgEstado() != null) {
            return false;
        }

        if (getDsBandeira() != null && e.getDsBandeira() == null
                || getDsBandeira() == null && e.getDsBandeira() != null) {
            return false;
        }

        if (getCodCupom() != null && e.getCodCupom() == null
                || getCodCupom() == null && e.getCodCupom() != null) {
            return false;
        }

        if (!getDescUser().equals(e.getDescUser())) {
            return false;
        }

        if (!getNmMother().equals(e.getNmMother())) {
            return false;
        }

        if (!getSgEstado().equals(e.getSgEstado())) {
            return false;
        }

        if (!getDsBandeira().equals(e.getDsBandeira())) {
            return false;
        }

        if (!getCodCupom().equals(e.getCodCupom())) {
            return false;
        }

        return true;
    }

}

1 Resposta 1

3

Odeio este tipo de ferramenta que coloca números. 15 é absurdamente alto em algumas situações e pouco em outros. Para arrumar isso teria que tornar o código espalhado (em vários métodos) sem necessidade, me parece que tudo o que está aí é uma coisa só. Eu calaria a boca deste software. Deve ter uma forma de fazer isso com configuração e/ou anotação.

O problema não é quantidade de if e sim de decisões a serem todas, não tem 34 if, mas tem 34 comparações que geram booleanos. Se quiser reduzir para apenas dois ifs é tranquilo porque todos geram o mesmo resultado, pra falar a verdade precisa de um ifs (em C# poderia se feito com zero).

Veja se atrapalha a visualização:

public boolean equals(Object o) {
    if (o == null || o.getClass() != getClass()) return false;
    final EntidadeGrande e = (EntidadeGrande) o;
    return (getCodUser() == e.getCodUser() && 
            getIdUser() == e.getIdUser() &&
            getScoreUser() == e.getScoreUser() &&
            getScoreStore() == e.getScoreStore() &&
            getCdLogo() == e.getCdLogo()) &&
            ((getValueX() == null || e.getValueX() != null) &&
                (getValueX() != null || e.getValueX() == null)) &&
                getValueX().equals(e.getValueX()) &&
            ((getDescUser() != null || e.getDescUser() == null) &&
                (getDescUser() == null || e.getDescUser() != null)) &&
                getDescUser().equals(e.getDescUser()) &&
            ((getNmMother() != null || e.getNmMother() == null) &&
                (getNmMother() == null || e.getNmMother() != null)) &&
                getNmMother().equals(e.getNmMother()) &&
            ((getSgEstado() == null || e.getSgEstado() != null) &&
                (getSgEstado() != null || e.getSgEstado() == null)) &&
                getSgEstado().equals(e.getSgEstado()) &&
            ((getDsBandeira() == null || e.getDsBandeira() == null) &&
                (getDsBandeira() == null || e.getDsBandeira() != null)) &&
                getDsBandeira().equals(e.getDsBandeira()) &&
            ((getCodCupom() == null || e.getCodCupom() != null) &&
                (getCodCupom() != null || e.getCodCupom() == null)) &&
                getCodCupom().equals(e.getCodCupom());
}

Coloquei no GitHub para referência futura.

Eu posso ter comido bola em algum, por isso não copie e cole.

Se não quiser calar este analisador terá que fazer o que ele manda e criar cada grupo de condições em métodos diferentes e depois criar métodos para colar tudo junto, ficando mais confuso e menos eficiente. Ridículo, mas cada um faz como achar melhor.

Se for por esse caminho eu iria pelos grupos que já criou, talvez reagrupando um pouco (você tinha começado bem), verifique se é null e se é igual junto, o que faz mais sentido até mesmo no caso de manter tudo em um método só, provavelmente será mais eficiente e mais legível.

Em C# eu faria de uma forma muito mais simples que isso, mas sei que Java gosta de código verboso e menos robusto, não sei dizer se poderia fazer bem melhor que isso como dá em C# com a arquitetura certa.

Dito tudo isso, é possível que isso realmente possa ser interessante estar em métodos separados que controlam cada um dos campos individualmente, porque eles isoladamente devem ser tratados assim, ou sejam toda engenharia do software precisa ser pensada para lidar com isso, principalmente se cada condição possa ser usada novamente em algum outro local, afinal devemos seguir o DRY, ou seja, fazer a boa e velha abstração, que inclusive tem a ver com orientação a objeto que tanto defendem. Mas não tenho informação suficiente para saber se isto é o mais adequado neste caso. Certamente seria mais testável, o que eu questiono se deveria ser o objetivo principal do código, mas se for para mudar deve ser em primeiro lugar para atender a demanda do problema, só depois pensar no teste, e não pensar no SonarLint mal configurado.

Fico pensando se não tem problemas mais importante em todo o conceito adotado.

3
  • Obrigado, onde trabalho os códigos passam por um sonar corporativo automaticamente em todo push em um determinado branch, e dependendo da "nota" que o commit ganhar do sonar, esse push é recusado.
    – res
    Commented 13/08/2018 às 19:04
  • Essa classe eu eu criei apenas para ter um código mínimo e testável para postar aqui, não tem problemas faltar algo, era só para referência mesmo. E o jeito é ir separando em vários métodos de comparação, ou mesclar os IFs como você fez.
    – res
    Commented 13/08/2018 às 19:05
  • 4
    Eu adoro esses workflows que incentivam fazer código ruim porque uma porcaria de um software determinou, em tempos de inteligência artificial o TI ainda aplica muita falta de inteligência natural. Lamento por você ser obrigado fazer uma atrocidade dessas.
    – Maniero
    Commented 13/08/2018 às 19:07

Você deve fazer log-in para responder a esta pergunta.

Esta não é a resposta que você está procurando? Pesquise outras perguntas com a tag .