CODEWITHSUNDEEP
java
Sam B
Sam B

Reputation: 45

Compressing Similar if Statements (Java)

I am trying to select a certain array out of 16 depending on what value is passed into the constructor of a class. The value entered will be used in a series of if statements that will choose the array. Something like this

    package Model;

    import javax.swing.*;
    public class Die extends JButton{
         String letters[] = new String[6];
         public Die(int number){
               Controller con = new Controller();
               if(number==1){
                    for(int i = 0; i<5; i++}{
                        letters[i] = con.die1[i];
                    }
               }
               if(number==2){
                    for(int i = 0; i<5; i++}{
                        letters[i] = con.die2[i];
                    }
               }
               if(number==3){
                    for(int i = 0; i<5; i++}{
                        letters[i] = con.die3[i];
                    }
               }
         }
    }

(Keep in mind that I haven't checked this code, and I have not created all 16 if statements).
Basically I want to compress these if statements somehow. Any ideas?

Upvotes: 2

Views: 399

Answers (5)

davidxxx
davidxxx

Reputation: 131346

Declare a Map<Integer, String[]> that associates the number to the String to each die array and you would have not any conditional statements. 

Controller con = new Controller();
Map<Integer, String[]> map = new HashMap<>();   
map.put(1, con.die1);
map.put(2, con.die2);
map.put(3, con.die3);


if(number>=1 && number<=3){
    for(int i = 0; i<5; i++}{
        letters[i] = map.get(number)[i];
    }
}

And as suggested by Lino you could even move this mapping into the Controller class which makes more sense in terms of responsibility :

private Map<Integer, String[]> map = new HashMap<>();
public Controller(){
  map.put(1, die1);
  map.put(2, die2);
  map.put(3, die3);
}
public String getLetter(int number, int index){
    map.get(number)[index];
}

And the Die client could be now :

Controller con = new Controller();       
if(number>=1 && number<=3){
    for(int i = 0; i<5; i++}{
        letters[i] = con.getLetter(number, i);
    }
}

Upvotes: 5

Shababb Karim
Shababb Karim

Reputation: 3703

You can use Arrays.copyOf method from Java API.

               if(number==1){
                    letters = Arrays.copyOf(con.die1, 5); 
               }
               if(number==2){
                    letters = Arrays.copyOf(con.die2, 5); 
               }
               if(number==3){
                    letters = Arrays.copyOf(con.die3, 5); 
               }

Upvotes: 0

Kevin Cruijssen
Kevin Cruijssen

Reputation: 9326

Since all loops are the same, you could move the if-statement inside the loop. In addition, you could change the if-statements to a switch, or (perhaps less readable but shorter) a nested ternary-if:

With switch:

Controller con = new Controller();
for(int i=0; i<5; i++){
  switch(number){
    case 1:
      letters[i] = con.die1[i];
      break;
    case 2:
      letters[i] = con.die2[i];
      break;
    case 3:
      letters[i] = con.die3[i];
      break;
    ...
    default:
      // Do something if the number is unknown
      break;
  }
}

With ternary if:

Controller con = new Controller();
for(int i=0; i<5; i++){
  letters[i] = number==1 ? con.die1[i]
               : number==2 ? con.die2[i]
               : number==3 ? con.die3[i]
               : ...;
}

A ternary if is basically a shorter variation of an if-else (in the form of <condition> ? <value_if_true> : <else_value>. Here an example of a ternary if that is perhaps better understandable:

int a=...;
System.out.println(a<5 ? "Less than 5" : a<15 ? "Less than 15" : "Else (15 or larger)");

This is basically the same as:

if(a<5){
  System.out.println("Less than 5");
} else if(a<15){
  System.out.println("Less than 15");
} else{
  System.out.println("Else (15 or larger)");
}

Try it online.

As you can see, it's much less readable, so I would prefer to use longer if-statements in most cases, but with your code I would personally use a ternary if to make the code more compact without making it too complex.

Upvotes: 1

Lutz B&#252;ch
Lutz B&#252;ch

Reputation: 363

I recommend the switch statement:

String[] array;

switch (number) {
        case 1:  array = con.die1;
                 break;
        case 2:  array = con.die2;
                 break;
        [...]
        case 16: array = con.die16;
                 break;
        default: // throw error or select a default array
    }

Upvotes: 0

Lino
Lino

Reputation: 19926

Best would be to select in the if branches only which array that should be iterated:

String[] array;

if(number == 1){
    array = con.die1;
} else if(number == 2){
    array = con.die2;
} else if(number == 3){
    array = con.die3;
} else {
    // throw error or select a default array
}

And then just iterate over the selected array afterwards outside of the ifs:

for(int i = 0; i<5; i++}{
     letters[i] = array[i];
}

For future cases, it is best to analyze the code inside the if-statements.

  1. What is the same everywhere?
  2. What is different everywhere?

Just from these 2 points you'll see that:

  1. You're always iterating over an array.
  2. The array is just different everytime.

So it is quite easy to see that just extracting the logic of iterating out of the if-statements. And the selection of the array to stay inside.

Upvotes: 4

Related Questions