Java simplifying repetitive code involving boolean comparison

Question

I am trying to find a way to reduce the length and simplify the following repetitive methods :

boolean circleFlag, squareFlag, diamondFlag;

public void shapeButtonPressed(String shapeType) {

    if (shapeType.equals("Circle")) {
        circlePressed();
    } else if (shapeType.equals("Square")) {
        squarePressed();
    } else if (shapeType.equals("Diamond")) {
        diamondPressed();   
    }

}

public void circlePressed() {
    if(!circleFlag){
        //set only circleFlag true and the rest false.
        circleFlag = true;
        squareFlag = false;
        diamondFlag = false;

        //(... some code)
    } else {
        //set all flags false.
        circleFlag = false; 
        diamondFlag = false
        squareFlag = false;

        //(... some different code)
    }

}
public void squarePressed() {
    if(!squareFlag){
        //set only squareFlag true and the rest false.
        squareFlag = true;
        circleFlag = false;
        diamondFlag = false;

        //(... some code)
    } else {
        //set all flags false.
        circleFlag = false; 
        diamondFlag = false
        squareFlag = false;

        //(... some different code)
    }

}
public void diamondPressed() {
    if(!diamondFlag){
        //set only diamondFlag true and the rest false.
        diamondFlag = true;
        squareFlag = false;
        circleFlag = false;

        //(... some code)
    } else {
        //set all flags false.
        circleFlag = false; 
        diamondFlag = false
        squareFlag = false;

        //(... some different code)
    }
}

---

Things I have tried

I have tried to set all my values to Boolean type, set them in a ArrayList<Boolean> and change the shapePressed(String shapeType) method to

public void shapePressed(String shapeType) {

    Boolean currFlag = false;

    if (shapeType.equals("Circle")) {
        currFlag = circleFlag;
    } else if (shapeType.equals("Square")) {
        currFlag = squareFlag;
    } else if (shapeType.equals("Diamond")) {
        currFlag = diamondFlag;
    }

    if (!currFlag){
        for (Boolean flag : shapeFlag) flag = ( flag == currFlag ) ? true : false; 

        //(...)
    } else {
        for (Boolean flag : shapeFlag) flag = false;

        //(...)
    }
}

but the line ( flag == currFlag ) compares the booleans as values and not as individual objects. So my currFlag is pointless in this above method.

I then though of using a HashMap<String ,Boolean> but whenever I compare the values given a key (String shapeType from the method parameter), I encounter the same problem as above.

What is a way to simplify this code ?

Answer 1

As an alternative to my approach with enums above (that I highly recommend over this one), you can do a more "C-style" solution using a bitmask instead of boolean flags.

A bitmask is essentially a numeric (or binary for that matter) value of which each bit represents a boolean value.

int shapeFlags;

public void shapeButtonPressed(String shapeType) {
    if (shapeType.equals("Circle")) {
        shapeFlags = 1;
    } else if (shapeType.equals("Square")) {
        shapeFlags = 2;
    } else if (shapeType.equals("Diamond")) {
        shapeFlags = 4;
    }
}

This still leaves you the option to set more than one shape to true while being able to override all flags in a single operation.

Mappings from numeric values to shapes would look like this:

0 : no shape
1 : circle
2 : square
3 : circle & square
4 : diamond
5 : diamond & circle
6 : diamond & square
7 : all three

Answer 2

When a given shape is activated, you just invert that flag. Then the other flags get set to false.

So, trivially, you could simplify your circlePressed() logic to:

public void circlePressed() {
    circleFlag = !circleFlag;
    squareFlag = false;
    diamondFlag = false;

}

Of course there's still a lot of repetition. You could refactor this further to an enum and track the state there.

public enum Flag {
  CIRCLE( false ),
  SQUARE( false ),
  DIAMOND( false ); // default state is false for all

  private boolean state;
  private Flag(boolean state) {
    this.state = state;
  }

  public void flipState() {
    this.state = !this.state;
  }

  public void setState(boolean state) {
    this.state = state;
  }
}

// notice this method takes the Flag not a string
public void shapeButtonPressed(Flag selected) {

    // iterate through all the flags ...
    for( Flag flag : Flag.values() ) {
        if (flag == selected) {
            // invert the "pressed" flag state
            flag.flipState();
        } else {
            // ... and set the rest to false
            flag.setState(false);
        }
    }
}

The built-in values method on enums returns a list of all of the defined enums, so we can just iterate across them.

It's a bit gimmicky, I admit, since it's not really what enums are intended for, but it simplifies your logic quite a bit.

Answer 3

You could use an enum.

public enum Shape {
  CIRCLE, SQUARE, DIAMOND
}

Then, use that in your code like so;

Shape shape;

public void shapeButtonPressed(Shape selectedShape) {
    shape = selectedShape;
}

If you can't change the method signature of shapeButtonPressed and it has to take a String, you can stil do

Shape shape;

public void shapeButtonPressed(String shapeType) {
    if (shapeType.equals("Circle")) {
        shape = Shape.CIRCLE;
    } else if (shapeType.equals("Square")) {
        shape = Shape.SQUARE;
    } else if (shapeType.equals("Diamond")) {
        shape = Shape.DIAMOND;
    }

}