Returning an object. Any code smell here?

Question

Returning an object. Any code smell here?

Navigation

#1 by (1 votes)
#2 by (0 votes)

1

I thought of the following public method:

public RespostaDeArme armar();

The class RespostaDeArme would look like this:

public class RespostaDeArme {

    private final ResultadoDeArme resultado;
    private final Set<Integer> zonasAbertas;

    public RespostaDeArme(ResultadoDeArme resultado) {
        this(resultado, new HashSet<>());
    }

    public RespostaDeArme(ResultadoDeArme resultado, Set<Integer> zonasAbertas) {
        this.resultado = resultado;
        this.zonasAbertas = new HashSet<>(zonasAbertas);
    }

    public ResultadoDeArme getResultado() {
        return resultado;
    }

    public Set<Integer> getZonasAbertas() {
        return zonasAbertas;
    }
}

public enum ResultadoDeArme {
    SUCESSO(1),
    ARME_RECUSADO_PARTICAO_JA_ARMADA(2),
    ARME_RECUSADO_EXISTEM_ZONAS_ABERTAS(3),
    ERRO(4);

    private final int numero;

    private ResultadoDeArme(int numero) {
        this.numero = numero;
    }

    public int getNumero() {
        return numero;
    }
}

It only makes sense to call the getZonasAbertas() method when the result is ARME_RECUSADO_EXISTEM_ZONAS_ABERTAS .

Question: does this code have any code smell ? The only thing I thought was a possible need to refactor the class containing the armar() method with Hide Delegate to not need the intermediate RespostaDeArme to get the zones open (which I do not know would be the case here).

Does anyone see a better way to implement this behavior? If necessary I give more context. I do not like the idea of having two methods in the same class, a armar() and a getZonasAbertas() , since the two would have an unnecessary time link (the programmer would need to know to call armar() first, then the other).

Example usage:

public void executarArme(int idDaCentral) {
    RespostaDeArme resposta = conexao.armar();
    escritorDaFila.escrever(gerarJsonDeRespostaDeArme(idDaCentral, resposta));
}

private JSONObject gerarJsonDeRespostaDeArme(int idDaCentral, RespostaDeArme resposta) {

    JSONObject json = new JSONObject();

    json.put("tipoDeMensagem", Mensagens.RESPOSTA_DE_ARME.getNumero());
    json.put("idDaCentral", idDaCentral);
    json.put("resultado", resposta.getResultado().getNumero());

    if (resposta.getResultado() == ResultadoDeArme.ARME_RECUSADO_EXISTEM_ZONAS_ABERTAS) {
        json.put("zonasAbertas", new JSONArray(resposta.getZonasAbertas()));
    }

    return json;
}

java objetos return

asked by anonymous 16.02.2017 / 19:07

2 answers

0

It depends a lot on the context in which the armar() method will be used. However, as it is, I believe that there is a code smell in fact, although it should not be a serious code smell. As far as I understand, a RespostaDeArme that has a ResultadoDeArme other than SUCESSO indicates that the armar() method failed. The correct mechanism for denoting such failures is handling exceptions.

So your code looks like this:

public class ArmeException extends Exception {}

public class ParticaoJaArmadaException extends ArmeException {}

public class ExistemZonasAbertasException extends ArmeException {

    private final Set<Integer> zonasAbertas;

    public RespostaDeArme(Set<Integer> zonasAbertas) {
        this.zonasAbertas = new HashSet<>(zonasAbertas);
    }

    public Set<Integer> getZonasAbertas() {
        return zonasAbertas;
    }
}

public class ArmeFalhouException extends ArmeException {}

And your armar() method looks like this:

public void armar() throws
        ParticaoJaArmadaException,
        ExistemZonasAbertasException,
        ArmeFalhouException;

16.02.2017 / 19:23

Specific Url in Django Convert a winform to webform

score 1 · Accepted Answer

Now that you have edited the question, this can be resolved with a simple Optional , hiding the constructor and exposing static methods of factory :

public final class RespostaDeArme {

    private final ResultadoDeArme resultado;
    private final Optional<Set<Integer>> zonasAbertas;

    public static RespostaDeArme sucesso() {
        return new RespostaDeArme(ResultadoDeArme.SUCESSO, Optional.empty());
    }

    public static RespostaDeArme armeFalhou() {
        return new RespostaDeArme(ResultadoDeArme.ERRO, Optional.empty());
    }

    public static RespostaDeArme particaoJaArmada() {
        return new RespostaDeArme(
                ResultadoDeArme.ARME_RECUSADO_PARTICAO_JA_ARMADA,
                Optional.empty());
    }

    public static RespostaDeArme existemZonasAbertas(Set<Integer> zonasAbertas) {
        return new RespostaDeArme(
                ResultadoDeArme.ARME_RECUSADO_EXISTEM_ZONAS_ABERTAS,
                Optional.of(Collections.unmodifiableSet<>(new LinkedHashSet<>(zonasAbertas))));
    }

    private RespostaDeArme(ResultadoDeArme resultado, Optional<Set<Integer>> zonasAbertas) {
        this.resultado = resultado;
        this.zonasAbertas = new HashSet<>(zonasAbertas);
    }

    public ResultadoDeArme getResultado() {
        return resultado;
    }

    public int getNumeroResultado() {
        return resultado.getNumero();
    }

    public Optional<Set<Integer>> getZonasAbertas() {
        return zonasAbertas;
    }
}

public enum ResultadoDeArme {
    SUCESSO,
    ARME_RECUSADO_PARTICAO_JA_ARMADA,
    ARME_RECUSADO_EXISTEM_ZONAS_ABERTAS,
    ERRO;

    public int getNumero() {
        return ordinal() + 1;
    }
}

public void executarArme(int idDaCentral) {
    RespostaDeArme resposta = conexao.armar();
    escritorDaFila.escrever(gerarJsonDeRespostaDeArme(idDaCentral, resposta));
}

private JSONObject gerarJsonDeRespostaDeArme(int idDaCentral, RespostaDeArme resposta) {

    JSONObject json = new JSONObject();

    json.put("tipoDeMensagem", Mensagens.RESPOSTA_DE_ARME.getNumero());
    json.put("idDaCentral", idDaCentral);
    json.put("resultado", resposta.getNumeroResultado());

    resposta.getZonasAbertas().ifPresent(zonas -> {
        json.put("zonasAbertas", new JSONArray(zonas));
    });

    return json;
}