Правильный способ рефрактора if-утверждений

Question

Интересно, это хороший способ для рефакторинга нескольких операторов if?Я слышал, что операторы if - это «плохая» практика, и я хотел бы поднять свое программирование на новый уровень путем рефакторинга части моего кода.

Оригинальный код:

public int Colors(String col, int row){
    int ret = 0;
    if(col.equals("R")){
        ret = Color.parseColor("#EF5350");
    }else if(col.equals("B")) {
        ret = Color.parseColor("#7986CB");
    }else if(col.equals("V")){
        ret = Color.WHITE;
    }else if(col.equals("G")){
     ret = Color.parseColor("#FFE082");
    }else if(Math.floorMod(row,2) == 0){
        ret = Color.parseColor("#e0e0e0");
    }else{
         ret = Color.parseColor("#eeeeee");
    }

return ret;}

Новый код:

public int Colors(String col, int row){

    Map<String,Integer> ColorMap1 = new HashMap<>();

    ColorMap1.put("R", Color.parseColor("#EF5350"));
    ColorMap1.put("B",Color.parseColor("#7986CB"));
    ColorMap1.put("V",Color.parseColor("#FFE082"));
    ColorMap1.put("G",Color.parseColor("#FFFFFF"));

    Integer current = ColorMap1.get(col);

    Map<Integer,Integer> ColorMap2 = new HashMap<>();

    ColorMap2.put(0,Color.parseColor("#e0e0e0"));
    ColorMap2.put(1,Color.parseColor("#eeeeee"));

    Integer current2 = ColorMap2.get(Math.floorMod(row,2));

    return  current != null ? current  : current2 ;

}

Answer 1

Вы можете использовать enum для добавления разных цветов:

public class Main
{
    public static void main(String[] args) {
        String colorName = "R";
        int row = 23;
        System.out.println("Color code: " + ColorSchema.getColor(colorName, row).getCode());
    }
}


enum ColorSchema {

  R("#EF5350"), 
  B("#7986CB"), 
  V("#FFE082"),
  G("#FFFFFF"),

  ROW0("#e0e0e0"),
  ROW1("#eeeeee");

  private String color;

  ColorSchema(String color) {this.color = color;}
  public int getCode(){ return Color.parseColor(color);}

  public static ColorSchema getColor(String name, int row) {
    for (ColorSchema c : ColorSchema.values()) {
      if (c.color.equals(name)) {
        return c;
      }
    }
    return ColorSchema.valueOf("ROW" + Math.floorMod(row, 2));
  }

}

Answer 2

If утверждение - плохая практика, если вы использовали его чрезмерно и в разных местах.Лучший способ рефакторинга - использовать шаблон проектирования. Вместо оператора if можно использовать множество шаблонов, например strategy, state или command шаблон

.

Answer 3

if в целом неплохая практика.Просто, когда вы обнаруживаете, что используете несколько цепочек if - else if подряд, это обычно запах кода, который указывает, что вы можете делать то, что делаете, более эффективно.

В этом случае, используястатический Map - это хороший способ реорганизовать большую часть кода, чтобы он был более эффективным, чем эквивалент if.Проблема в том, что одно из условий не соответствует остальным и не является постоянным значением.Таким образом, вы можете частично реорганизовать свой код следующим образом:

static HashMap<String, Color> colorMap;

static {
    colorMap = new HashMap<>();
    colorMap.put("R",Color.parseColor("#EF5350"));
    colorMap.put("B",Color.parseColor("#7986CB"));
    colorMap.put("V",Color.parseColor("#FFE082"));
    colorMap.put("G",Color.parseColor("#FFFFFF"));
}

// ...

public int Colors(String col, int row) {
    if (colorMap.containsKey(col)) {
      return colorMap.get(col);
    }

    if (Math.floorMod(row, 2) == 0) {
      return Color.parseColor("#e0e0e0");
    }

    return Color.parseColor("#eeeeee");
}

В качестве альтернативы, вы можете выполнить аналогичное действие с помощью блока switch:

public int Colors(String col, int row) {
    switch (col) {
        case "R": return Color.parseColor("#EF5350");
        case "B": return Color.parseColor("#7986CB");
        case "V": return Color.parseColor("#FFE082");
        case "G": return Color.parseColor("#FFFFFF");
    }

    if (Math.floorMod(row, 2) == 0) {
      return Color.parseColor("#e0e0e0");
    }

    return Color.parseColor("#eeeeee");
}

Answer 4

Можно утверждать, что if-statements или switch-statements всегда плохие.Иногда они приводят к гораздо более лаконичному коду.Но если число случаев становится действительно высоким, тогда становится понятнее использовать Map.

. Вы можете использовать реестр карт для цветов следующим образом:

public class SomeClass {
    private static int DEFAULT_COLOR = Color.parseColor("#eeeeee");
    private static Map<ColorKey, Integer> COLORS = new HashMap<>();
    static {
        COLORS.put(ColorKey.of("R", null), Color.parseColor("#EF5350"));
        COLORS.put(ColorKey.of("B", null), Color.parseColor("#7986CB"));
        COLORS.put(ColorKey.of("V", null), Color.WHITE);
        COLORS.put(ColorKey.of("G", null), Color.parseColor("#FFE082"));
        COLORS.put(ColorKey.of(null, 0), Color.parseColor("#e0e0e0"));
    }

    public Integer colors(String col, int row) {
        return COLORS.getOrDefault(ColorKey.of(col, row), DEFAULT_COLOR);
    }

    private static final class ColorKey {
       private final String col;
       private final Integer row;
       private ColorKey(String col, Integer row) {
           this.col = col; this.row = row;
       }
       private static ColorKey of(String col, Integer row) {
           return new ColorKey(key(col), key(row));
       }
       private static String key(String col) {
           Set<String> columns = new HashSet<>(Arrays.asList("R", "B", "V", "G"));
           return columns.contains(col) ? col : null;
       }
       private static Integer key(Integer row) {
           return row != null && Math.floorMod(row,2) == 0 ? Integer.valueOf(0) : null;
       }
       @Override
       public boolean equals(Object obj) {
           if (obj == null) return false;
           if (obj == this)  return true;
           if (!(obj instanceof ColorKey)) return false;
           ColorKey that = (ColorKey) obj;
           return Objects.equals(this.col, that.col) && Objects.equals(this.row, that.row);
       }
       @Override
       public int hashCode() {
           int result = 17;
           result = 31 * result + Objects.hash(col);
           result = 31 * result + Objects.hash(row);
           return result;
       }
    }
}

Answer 5

If-утверждения не являются по своей сути плохими.Они могут быть немного неуклюжими для больших блоков сравнения, но в Java нет универсальной альтернативы.(Вы можете использовать when в Kotlin.)

Поскольку у вас есть большой блок и вы сравниваете строки, вы всегда можете использовать оператор switch, для большей части из него:

public int colors(String col, int row) //method names in Java should usually use camelCase, not TitleCase
    switch(col) {
        case "R":
            return Color.parseColor("#EF5350");
        case "B":
            return Color.parseColor("#7986CB");
        case "V":
            return Color.WHITE;
        case "G":
            return Color.parseColor("#FFE082");
        default: //comparable to an "else" statement
            if (Math.floorMod(row, 2) == 0) return Color.parseColor("#e0e0e0");
            else return Color.parseColor("#eeeeee");
    }
}