CODEWITHSUNDEEP
javaif-statement
Antoine
Antoine

Reputation: 75

Reduce the number of if statements, or replace by a for or switch or other

My if and else if conditionals are too long. Is it possible to put a for or while or switch or any other function that could reduce the code and make it more understandable?

Apart from that, the code is running fine.

if((text.charAt(7)=='O' || text.charAt(7)=='X')
        && (text.charAt(8)=='O' || text.charAt(8)=='X')
        && (text.charAt(9)=='O' || text.charAt(9)=='X') 
        && (text.charAt(10)=='O'|| text.charAt(10)=='X')
        && text.charAt(49)=='O'
        && text.charAt(50)=='O'
        && text.charAt(51)=='X')
 {
    ledGeneral.setBackgroundResource(R.drawable.ledverte);
}else if (text.charAt(7)=='A' 
        || text.charAt(8)=='A'
        || text.charAt(9)=='A'
        || text.charAt(10)=='A'
        || text.charAt(49)=='A'
        || text.charAt(50)=='A')
{
    ledGeneral.setBackgroundResource(R.drawable.ledgeneralrouge);
}else{
    ledGeneral.setBackgroundResource(R.drawable.imageverte);
}

Upvotes: 2

Views: 164

Answers (4)

Eritrean
Eritrean

Reputation: 16498

If the indices you are checking are fixed:

if(text.matches(".{7}[OX][OX][OX][OX].{38}OOX.*")){
    ledGeneral.setBackgroundResource(R.drawable.ledverte);
}
else if (text.substring(7, 11).contains("A") || text.substring(49, 51).contains("A")){
    ledGeneral.setBackgroundResource(R.drawable.ledgeneralrouge);
}
else{
    ledGeneral.setBackgroundResource(R.drawable.imageverte);
}

{38} and also the {7} are so called quantifiers. Matches the specified quantity of the previous token. Example:

  • [X]{1,3} will match 1 to 3 X's.
  • {3} will match exactly 3.
  • {3,} will match 3 or more.

Generally [x]{min,max} : to match min to max x's, [x]{n} to match exactly n x's and [x]{min,} to match atleast min x's or more.

And the dot . means match any char. So .{38} means match any char exactly 38 times

As your condition can be interpreted as:

match a string which

  • starts with any 7 chars (index 0 - 6)
  • and a O or X at the 7th, 8th, 9th and 10th indexes
  • and any char from 11th to the 48th index (38 chars)
  • and a O at 49th and 50th index
  • and a X at 51th index

connverted into regular expression

  • ....... or shorter .{7} (any char exactly 7 times)
  • [OX][OX][OX][OX] one of the chars specified in the char class which correspond to your indices 7,8,9,10
  • ....................................... or shorter .{38} (any char exactly 38 times)
  • followed by exactly OOX at the indices 49,50,51
  • .* optional zero or more additional chars at the end

So the long version could be:

.......[OX][OX][OX][OX]......................................OOX.*

Shorter

.{7}[OX][OX][OX][OX].{38}OOX.*

Upvotes: 1

dreamcrash
dreamcrash

Reputation: 51413

You can create a auxiliary function to deal with that:

static boolean check_letter(String text, char to_check, int ...pos){
    for (int po : pos) {
        if (text.charAt(po) == to_check)
            return true;
    }
    return false;
}

then adapt the if and else:

if((check_letter(text, 'O', 7, 8, 9, 10) || check_letter(text, 'X', 7, 8, 9, 10)) && text.charAt(49) == 'O' && text.charAt(50) == 'O' && text.charAt(51) == 'X')
{
  ...
}
else if (check_letter(text, 'A', 7, 8, 9, 10, 49, 50))
{
 ...
}

Upvotes: 5

Haris Bouchlis
Haris Bouchlis

Reputation: 2566

For the part of the question about code "cleanliness", I'd suggest moving your statements in descriptive methods e.g.

private boolean shouldSetBackgroundLedverte(text){
  return (text.charAt(7)=='O' || text.charAt(7)=='X') && (text.charAt(8)=='O'|| 
         text.charAt(8)=='X')&& (text.charAt(9)=='O'|| text.charAt(9)=='X') && 
         (text.charAt(10)=='O'|| text.charAt(10)=='X')&& text.charAt(49)=='O'&& 
         text.charAt(50)=='O'&& text.charAt(51)=='X';
}

private boolean shouldSetBackgroundLedgeneralrouge(text){
  return text.charAt(7)=='A' || text.charAt(8)=='A'|| text.charAt(9)=='A'|| 
         text.charAt(10)=='A'|| text.charAt(49)=='A'|| text.charAt(50)=='A'
}

private setBackground(String text) {
  if (shouldSetBackgroundLedverte(text)) {
    ledGeneral.setBackgroundResource(R.drawable.ledverte);
  } else if (shouldSetBackgroundLedgeneralrouge(text)) {
    ledGeneral.setBackgroundResource(R.drawable.ledgeneralrouge);
  } else {
    ledGeneral.setBackgroundResource(R.drawable.imageverte);
  }
}

Upvotes: 1

Smutje
Smutje

Reputation: 18123

You could try the following although I doubt that this makes it more readable:

final boolean firstCondition = IntStream.of(7, 8, 9, 10)
        .mapToObj(text::charAt)
        .allMatch(character -> character == 'O' || character == 'X')
          && text.charAt(49) == 'O'
          && text.charAt(50) == 'O'
          && text.charAt(51) == 'X';
final boolean secondCondition = IntStream.of(7, 8, 9, 10, 49, 50)
        .mapToObj(text::charAt)
        .anyMatch(character -> character == 'A');

if (firstCondition) {
    //
} else if (secondCondition) {
    //
} else {
    //
}

Upvotes: 0

Related Questions