CODEWITHSUNDEEP
javarefactoring
androidcodehunter
androidcodehunter

Reputation: 21937

How to Refactor this type of switch-case statement?

Based on the minIndex, I am setting the different view visibility. I think it can be refactored as simple way.

switch (minIndex) {
    case 0:
        viewOne.setVisibility(View.VISIBLE);
        break;
    case 1:
        viewTwo.setVisibility(View.VISIBLE);
        break;
    case 2:
        viewThree.setVisibility(View.VISIBLE);
        break;
    case 3:
        viewFour.setVisibility(View.VISIBLE);
        break;
    case 4:
        viewFive.setVisibility(View.VISIBLE);
        break;
    case 5:
        viewSix.setVisibility(View.VISIBLE);
        break;
}

How can I refactor this code as a more readable code?

Upvotes: 1

Views: 336

Answers (2)

Eric Jablow
Eric Jablow

Reputation: 7899

I assume you've anonymized your code for this site. I was going to suggest what @Sotirios Delimanolis answered, but I think you are actually doing both too much and too little.

You've bound the controller too much to the view, and you've bound the views too much to each other. Why should all of them be in the same switch statement?

How much will you need to change if you add another view?

Instead, you should have each view register a PropertyChangeListener with the object that holds minindex. That's a bad name, by the way. When the object changes minindex, it should send a PropertyChangeEvent to all the listeners. Each view's listener should check whether the event wants that view to become visible; if so, the view should awaken itself.

class ViewController {
    private PropertyChangeSupport pcs = new PropertyChangeSupport();

    // delegate methods to add and remove listeners to pcs variable.

    private int viewIndex; // Changed for documentation.  Use String instead?

    public void setViewIndex(final int viewIndex) {
        int oldIndex = this.viewIndex;
        this.viewIndex = viewIndex; 
        pcs.firePropertyChange("viewIndex", oldIndex, this.viewIndex);
    }
}

class ViewOne {
    private ViewController vc;
    private final Integer myIndex = 1;

    // Constructor

    public void init() {
        // Never add this or an inner class to another object in a constructor.
        vc.addPropertyChangeListener("viewIndex",
            new PropertyChangeListener() {
                public propertyChange(final PropertyChangeEvent evt) {
                    if (myIndex.equals(evt.getNewValue()) {
                         setVisibility(View.VISIBLE);
                    }
                }
            });
    }
}

The warning about constructors is that if you expose this or an inner class of this to another object in the constructor, that external object may interact with this before it is fully constructed. You can build the PCL in the constructor; use another method to add it to the controller.

Upvotes: 1

Sotirios Delimanolis
Sotirios Delimanolis

Reputation: 279970

If the numbers match up nicely with the actual view, you can use an array.

View[] views = new View[] {viewOne, viewTwo, viewThree, ...};
...
views[minIndex].setVisibility(View.VISIBLE);

Upvotes: 5

Related Questions