見やすいif文の書き方について

Question

#実現したいこと
カラーセンサーからの値によって、現在何色なのかを判別するif文を書いているのですが、
条件が多くなってしまい、汚いif文になってしまいました。
きれいに書く方法はありますでしょうか？
colorOutputはenumです。
よろしくお願いします。

#現在のコード
####分岐部分

      if(detectedColor.red>=0.2
      &&detectedColor.red<0.4
      &&detectedColor.green>=0.45
      &&detectedColor.blue<0.2){
        //yellow actions
        colorOutput = ColorThing.yellow;
      }else if(detectedColor.red>=0.3
      &&detectedColor.green>=0.2
      &&detectedColor.green<0.48
      &&detectedColor.blue<0.3){
        //red actions
        colorOutput = ColorThing.red;
      }else if(detectedColor.red<0.25
      &&detectedColor.green>=0.4
      &&detectedColor.blue>=0.2
      &&detectedColor.blue<0.27){
        //green actions
        colorOutput = ColorThing.green;
      }else if (detectedColor.red<0.25
      &&detectedColor.green>=0.4
      &&detectedColor.blue>=0.27){
        //blue actions
        colorOutput = ColorThing.blue;
      }else {
        //actions when unreadable
        colorOutput = ColorThing.cantRead;
      }

####enum部分

public enum ColorThing {
    yellow,
    red,
    green,
    blue,
    cantRead,
    outOfRange
  };
ColorThing colorOutput = ColorTHing.cantRead;

Comments

[m.ts10806]

なんでしょう「きれい」って。
個人の感覚によりすぎると得たいアドバイスは得られないと思います。
せめて方向性は指示してください

Answer 1

メソッドにしてはいかがですか？
ColorThingのメソッドにしてみました。

class Color {
  double red, green, blue;

  Color(double red, double green, double blue) {
      this.red = red;
      this.green = green;
      this.blue = blue;
  }
}

enum ColorThing {
  yellow, red, green, blue, cantRead, outOfRange;

  public static ColorThing from(Color color) {
    if (inRange(color, 0.2, 0.4, 0.45, 1.01, 0.0, 0.2)) {
      return yellow;
    } else if (inRange(color, 0.3, 1.01, 0.2, 0.48, 0.0, 0.3)) {
      return red;
    } else if (inRange(color, 0.0, 0.25, 0.4, 1.01, 0.2, 0.27)) {
      return green;
    } else if (inRange(color, 0.0, 0.25, 0.4, 1.01, 0.27, 1.01)) {
      return blue;
    } else {
      return cantRead;
    }
  }

  private static boolean inRange(Color color,
                                  double redMin,   double redOver,
                                  double greenMin, double greenOver,
                                  double blueMin,  double blueOver) {
    return inRange(color.red,   redMin,   redOver)
        && inRange(color.green, greenMin, greenOver)
        && inRange(color.blue,  blueMin,  blueOver);
  }

  private static boolean inRange(double color, double min, double over) {
    return min <= color && color < over;
  }
};

public class Main {

  public static void main(String[] args) {
    Color detectColor = new Color(0.2, 0.5, 0.7);
    ColorThing colorThing = ColorThing.from(detectColor);
    System.out.println(colorThing);
  }
}

Answer 2

まずメソッドに切り出すだけでもかなり読み易くなるように思います。
後はこんな工夫ができそうです。

• 早期returnの活用 (elseの排除)
• 比較演算子の向きを揃える
• 冗長でも意味が分かり易くなるならば括弧を使う

// どのクラスに置くと分かり良いかは設計に依ります。
// 場合に依ってはColorThingに持たせても良いかも。
private static ColorThing selectColorAction(double r, double g, double b) {
    if((0.2 <= r && r < 0.4) && (0.45 <= g) && (b < 0.2)) {
        return ColorThing.yellow;
    }
    if((0.3 <= r) && (0.2 <= g && g < 0.48) && (b < 0.3) {
        return ColorThing.red;
    }
    ...

    // 例外を投げるようにしても良いかもしれません。
    return ColorThing.cantRead;
}

colorOutput = selectColorAction(
    detectedColor.red, detectedColor.green, detectedColor.blue
);

Comments

[teityann1225]

修正してみました。

private static ColorCode selectColorAction(double r, double g, double b,int p){
if (p<80){
return ColorCode.outOfRange;
}
if ((0.2 <= r && r < 0.4) && (0.45 <= g) && (b < 0.2)){
return ColorCode.yellow;
}
if ((0.3 <= r) && (0.2 <= g && g < 0.48) && (b < 0.3)){
return ColorCode.red;
}
if ((r < 0.25) && (0.4 <= g) && (0.2 <=b && b < 0.27)){
return ColorCode.green;
}
if ((r < 0.25) && (0.4 <= g) && (0.27 <= b)){
return ColorCode.blue;
}
return ColorCode.inRange;
}

[LouiS0616]

既出の回答（私のものも・私以外のものも）で問題無いのであればBAを選んで解決済みにして下さい。
そうでないのであれば具体的に何が都合に合わないのか書いて下さい。

あるいはもっと回答が出揃うまで様子を見たいのでしょうか。

Answer 3

私はｘ軸が揃っているコードが見やすいです。

static ColorThing getInstance( double r, double g, double b ) 
{
   if( 0.2 <= r   && r   < 0.4  && 0.45 <= g     && b < 0.2 )  return yellow;
   if( 0.3 <= r   && 0.2 <= g   && g    <  0.48  && b < 0.3 )  return red;
   if( r   < 0.25 && 0.4 <= g   && 0.2  <= b     && b < 0.27 ) return green;
   if( r   < 0.25 && 0.4 <= g   && 0.27 <= b)                  return blue;
  
   return cantRead;
}

Answer 4

条件の部分をどう書けば見易いのかは人それぞれかと思いますので, 私ならこれくらいで妥協(?)するかもしれません.

enum ColorThing {
  yellow,
  red,
  green,
  blue,
  cantRead,
  outOfRange;

  static ColorThing getInstance(double r, double g, double b) {
    if(0.2 <= r && r < 0.4 && 0.45 <= g && b < 0.2) return yellow;
    if(0.3 <= r && 0.2 <= g && g < 0.48 && b < 0.3) return red;
    if(r < 0.25 && 0.4 <= g && 0.2 <= b && b < 0.27) return green;
    if(r < 0.25 && 0.4 <= g && 0.27 <= b) return blue;
    return cantRead;
  }
};

public class Main {
  public static void main(String[] args) throws Exception {
    class DetectedColor {
      double red, green, blue;
      DetectedColor(double r, double g, double b) {
        red=r; green=g; blue=b;
      }
    }
    DetectedColor detectedColor = new DetectedColor(0.1, 0.7, 0.2);

    ColorThing colorOutput = ColorThing.getInstance(detectedColor.red, detectedColor.green, detectedColor.blue);

    System.out.println("colorOutput="+colorOutput);
  }
}