Is it wrong to use multiple cases for the same action on the switch?

12

Is it wrong to do this with switch ? I programmed it like that, is it wrong?

switch(estado){
case "AL":
case "BA":
case "CE":
case "MA":
case "PB":
case "PE":
case "PI":
case "RN":
case "SE":
//FAZ ALGO AQUI PARA TODOS OS CASES DO NORDESTE
break;
case "ES":
case "MG":
case "RJ":
case "SP":
//FAZ ALGO AQUI PARA TODOS OS CASES DO SUDESTE
break;
etc...
    
asked by anonymous 25.09.2017 / 21:22

3 answers

17

No, this is perfectly valid and usual if that's what you want.

If all of these states should perform the same action, that's the way it should be.

In general it will perform better than doing the same with if , otherwise it makes more sense in this case.

I would only indent and align a little better:

switch (estado) {
    case "AL":
    case "BA":
    case "CE":
    case "MA":
    case "PB":
    case "PE":
    case "PI":
    case "RN":
    case "SE":
        //FAZ ALGO AQUI PARA TODOS OS CASES DO NORDESTE
        break;
    case "ES":
    case "MG":
    case "RJ":
    case "SP":
        //FAZ ALGO AQUI PARA TODOS OS CASES DO SUDESTE
        break;
    etc...
}

Or

switch (estado) {
case "AL":
case "BA":
case "CE":
case "MA":
case "PB":
case "PE":
case "PI":
case "RN":
case "SE":
    //FAZ ALGO AQUI PARA TODOS OS CASES DO NORDESTE
    break;
case "ES":
case "MG":
case "RJ":
case "SP":
    //FAZ ALGO AQUI PARA TODOS OS CASES DO SUDESTE
    break;
etc...
}

In such a case I would never change compile time at run time. The list is defined during development, finite and small and has practically no changes, so this is correct. I even like other solutions presented, but not for this case. It is more code, more complex, less legible, less performatic, that is, it is just to be clever without producing advantages.

Of course you can create an abstraction that addresses what each state should do, but this usually leaves the code complex, so it's only worth it if it's complex, and that additional complexity helps manage the overall complexity .

Note that the control of the state itself must be very simple, which will have to be in specific classes, after all you can always add new behaviors that are related to the states but that are part of a different subsystem. One is used for marketing, another for tax books, etc.

    
25.09.2017 / 21:24
9

Approach with Map

Since your purpose is to set a different color for each region, then:

private static final Map<String, String> regioes = new HashMap<>(27);
private static final Map<String, Color> cores = new HashMap<>(27);

static {
    String[] norte = {"AM", "AP", "AC", "RO", "RR", "PA", "TO"};
    String[] sul = {"PR", "SC", "RS"};
    String[] sudeste = {"SP", "MG", "RJ", "ES"};
    String[] nordeste = {"BA", "SE", "AL", "PE", "PB", "RN", "CE", "PI", "MA"};
    String[] centroOeste = {"MS", "MT", "GO", "DF"};

    for (String s : norte) regioes.put(s, "norte");
    for (String s : sul) regioes.put(s, "sul");
    for (String s : sudeste ) regioes.put(s, "sudeste");
    for (String s : nordeste ) regioes.put(s, "nordeste");
    for (String s : centroOeste ) regioes.put(s, "centro-oeste");

    cores.put("norte", Color.RED);
    cores.put("sul", Color.GREEN);
    cores.put("centro-oeste", Color.YELLOW);
    cores.put("nordeste", Color.BLUE);
    cores.put("sudeste", Color.ORANGE);
}

private static Color corDoEstado(String sigla) {
    String regiao = regioes.get(sigla);
    return cores.get(regiao);
} 

And then in your code, you just have to do this:

algumaCoisa.setCor(corDoEstado(estado));

The advantages of this approach is that:

  • Mapping is built only once in class loading.
  • Mapping is reusable in other places, and you get rid of having to repeat that awful switch whenever you deal with states.

Object-oriented approach

However, this approach still has its problems. The main problem is that all of this is about strings oriented programming for dealing with these things like strings and not as objects they should be.

So, the ideal thing is to do something like this:

package com.example;
import java.awt.Color;

public enum RegiaoBrasileira {
    SUL(Color.GREEN),
    SUDESTE(Color.ORANGE),
    CENTRO_OESTE(Color.YELLOW),
    NORTE(Color.RED),
    NORDESTE(Color.BLUE);

    private final Color cor;

    private RegiaoBrasileira(Color cor) {
        this.cor = cor;
    }

    public Color getCor() {
        return cor;
    }

    public static List<EstadoBrasileiro> getEstados() {
        return LocalizaEstados.porRegiao(this);
    }
}
package com.example;
import java.util.Locale;
import static com.example.RegiaoBrasileira.*;
import com.example.util.StringUtils;

public enum EstadoBrasileiro {
    RIO_GRANDE_DO_SUL("RS", SUL),
    SANTA_CATARINA("SC", SUL),
    PARANÁ("PR", SUL),
    SÃO_PAULO("SP", SUDESTE),
    RIO_DE_JANEIRO("RJ", SUDESTE),
    MINAS_GERAIS("MG", SUDESTE),
    ESPÍRITO_SANTO("ES", SUDESTE),
    BAHIA("BA", NORDESTE),
    SERGIPE("SE", NORDESTE),
    ALAGOAS("AL", NORDESTE),
    PERNAMBUCO("PE", NORDESTE),
    PARAÍBA("PB", NORDESTE),
    RIO_GRANDE_DO_NORTE("RN", NORDESTE),
    CEARÁ("CE", NORDESTE),
    PIAUÍ("PI", NORDESTE),
    MARANHÃO("MA", NORDESTE),
    PARÁ("PA", NORTE),
    AMAPÁ("AP", NORTE),
    ACRE("AC", NORTE),
    AMAZONAS("AM", NORTE),
    RONDÔNIA("RO", NORTE),
    RORAIMA("RR", NORTE),
    TOCANTINS("TO", NORTE),
    MATO_GROSSO("MT", CENTRO_OESTE),
    MATO_GROSSO_DO_SUL("MS", CENTRO_OESTE),
    GOIÁS("GO", CENTRO_OESTE),
    DISTRITO_FEDERAL("DF", CENTRO_OESTE),

    private final String nome;
    private final String sigla;
    private final RegiaoBrasileira regiao;

    private EstadoBrasileiro(String sigla, RegiaoBrasileira regiao) {
        this.sigla = sigla;
        this.regiao = regiao;
        this.nome = StringUtils.toTitleCase(name().toLowerCase(Locale.ROOT).replace("_", " "));
    }

    public String getNome() {
        return nome;
    }

    public String getSigla() {
        return sigla;
    }

    public RegiaoBrasileira getRegiao() {
        return regiao;
    }
}
package com.example;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

class LocalizaEstados {

    private static final Map<String, EstadoBrasileiro> SIGLAS_POR_ESTADO = new HashMap<>(27);
    private static final Map<RegiaoBrasileira, List<EstadoBrasileiro>> ESTADO_POR_REGIAO = new HashMap<>(27);

    static {
        for (RegiaoBrasileira s : RegiaoBrasileira.values()) {
            ESTADO_POR_REGIAO.put(s, new ArrayList<>(9));
        }
        for (EstadoBrasileiro s : EstadoBrasileiro.values()) {
            SIGLAS_POR_ESTADO.put(s.sigla, s);
            ESTADO_POR_REGIAO.get(s.getRegiao()).add(s);
        }
        for (RegiaoBrasileira s : RegiaoBrasileira.values()) {
            ESTADO_POR_REGIAO.put(s, Collections.unmodifiableList(ESTADO_POR_REGIAO.get(s));
        }
    }

    private LocalizaEstados() {}

    static EstadoBrasileiro porSigla(String sigla) {
        return SIGLAS_POR_ESTADO.get(sigla);
    }

    static List<EstadoBrasileiro> porRegiao(RegiaoBrasileira regiao) {
        return ESTADO_POR_REGIAO.get(regiao);
    }
}
package com.example.util;

class StringUtils {

    // Fonte: https://stackoverflow.com/a/1086134/540552
    public static String toTitleCase(String input) {
        StringBuilder titleCase = new StringBuilder();
        boolean nextTitleCase = true;

        for (char c : input.toCharArray()) {
            if (Character.isSpaceChar(c)) {
                nextTitleCase = true;
            } else if (nextTitleCase) {
                c = Character.toTitleCase(c);
                nextTitleCase = false;
            }

            titleCase.append(c);
        }

        return titleCase.toString();
    }
}

It may seem like a great job, but:

  • You separate the concepts of Brazilian states and regions from other parts of the code.

  • All this is reusable.

  • If you need to change concepts of Brazilian states and regions (for example, add the concept of capital of each state), it is not difficult to change the enum.

  • State concepts, Brazilian regions and corresponding colors no longer pollute other parts of the code.

Conclusion

Using switch is a horrible thing. It tends to pollute the code making it confusing and difficult to maintain. Polymorphism and adequate tabulation of data in appropriate data structures are better alternatives to it in most cases.

It's true that this seems to be more complicated than using switch . The problem is that you seldom end up using just one, and rather you end up using two, three, five, twenty times those switch s scattered in a lot of places in the code.

So, always be aware of the XY problem .

    
25.09.2017 / 23:00
6

As already answered, it is valid and works.

My choice is usually for the aesthetic part of the code, in this case performance is not a concern at all.

Another approach (yes, with a lot more code to write):

ActionProvider.java:

public interface ActionProvider {

    public void takeAction(String state);

}

NordesteActionProvider.java:

public class NordesteActionProvider implements ActionProvider {

    public void takeAction(String state) {
        System.out.println("Nordeste taking action");       
    }

}

SudesteActionProvider.java:

public class SudesteActionProvider implements ActionProvider {

    public void takeAction(String state) {
        System.out.println("Sudeste taking action");        
    }

}

StateStrategy.java:

public enum StateStrategy {

    NORDESTE(new NordesteActionProvider(), Arrays.asList("AL", "BA", "CE", "MA", "PB", "PE", "PI", "RN", "SE")),
    SUDESTE(new SudesteActionProvider(), Arrays.asList("ES", "MG", "RJ", "SP"));

    private List<String> states;
    private ActionProvider provider;

    StateStrategy(ActionProvider strategy, List<String> states) {
        this.states = states;
        this.provider = strategy;
    }

    public static Optional<StateStrategy> strategy(final String state) {
        return Arrays.asList(values())
                .stream()
                .filter(s -> { return s.states.contains(state); })
                .findFirst();               
    }

    public ActionProvider getActionProvider() {
        return provider;
    }

}

Using:

String al = "AL";
StateStrategy strategy = StateStrategy.strategy(al).orElseThrow(() -> new RuntimeException("Estado inválido"));
strategy.getActionProvider().takeAction(al);

String sp = "SP";
strategy = StateStrategy.strategy(sp).orElseThrow(() -> new RuntimeException("Estado inválido"));
strategy.getActionProvider().takeAction(sp);

Note: to improve ...

    
25.09.2017 / 22:08