Hi there! Today we will see, based on a real example,
how we can banish ugly switch-cases and enums from the code with simple polymorphism. The example bellow has been written by my self a few months ago. At this time i was convinced that it was simple and good enough. Now i know it better and i'm convinced that it could be better, more flexible and maintainable with little effort but with huge effect. The following example handles states of buttons in a game field i'm programming.
UML
Interface EnableDisable
import java.util.List;
import javax.swing.JButton;
public interface EnableDisable {
public void disableButtons(List<JButton> gameFieldButtons, int... indizes);
public void disableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);
public void enableButtons(List<JButton> gameFieldButtons, int... indizes);
public void enableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);
}
Class EnableDisableUtil
import java.awt.Color;
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;
public class EnableDisableUtil implements EnableDisable {
public void disableButtons(List<JButton> gameFieldButtons, int... indizes) {
disableOrEnable(Choice.DISABLE, gameFieldButtons, indizes);
}
public void disableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
disableOrEnable(Choice.DISABLE, selectedButton, gameFieldButtons, steps);
}
public void enableButtons(List<JButton> gameFieldButtons, int... indizes) {
disableOrEnable(Choice.ENABLE, gameFieldButtons, indizes);
}
public void enableButtons(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
disableOrEnable(Choice.ENABLE, selectedButton, gameFieldButtons, steps);
}
private void disableOrEnable(Choice choice, List<JButton> gameFieldButtons, int... indizes) {
switch (choice) {
case ENABLE:
for (int i = 0; i < indizes.length; i++) {
enable(indizes[i], gameFieldButtons);
}
break;
case DISABLE:
for (int i = 0; i < indizes.length; i++) {
disable(indizes[i], gameFieldButtons);
}
break;
default:
break;
}
}
private void disableOrEnable(Choice choice, JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
for (int i = 0; i < steps.length; i++) {
int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
switch (choice) {
case ENABLE:
enable(indexFromButtonToBeDisabled, gameFieldButtons);
break;
case DISABLE:
disable(indexFromButtonToBeDisabled, gameFieldButtons);
break;
default:
break;
}
}
}
private void enable(int buttonIndex, List<JButton> gameFieldButtons) {
JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
buttonToBeDisabled.setEnabled(true);
buttonToBeDisabled.setBorder(new LineBorder(Color.red, 1, false));
}
private void disable(int buttonIndex, List<JButton> gameFieldButtons) {
JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
buttonToBeDisabled.setEnabled(false);
buttonToBeDisabled.setBorder(new LineBorder(Color.lightGray, 1, false));
}
private enum Choice {
ENABLE, DISABLE;
}
}
What are we doing in this example?
Well, here i'm disabling or enabling some gameFields from my application depending on the method i call. (public methods).
Now my question:
What's wrong with that? It seems to be pragmatic for this simple use case, right? Well although at first glance this use case seems plausible and functionally ok(it passes the unit test), this design has potential for improvement. Let's point out first what i did "wrong":
- The name of my interface is not smart. It implies more then one responsabilities.
- I repeat myself in the signature of the methods of EnableDisable. Only the parameters are different
- The enum and switches signals fix states to me. So it would be better to handle it over polymorphie.
Let's do the changes. First i wanna rename my interface from EnableDisable to Switchable and let's also eliminate 2 methods. In the sequence let's declare 2 classes called SwitchOn and SwitchOff.
Nice! Thats a lot more interesting. But let's take a look at the concrete classes
SwitchOn and
SwitchOff to be able to decide if it is good this way or not.
import java.util.List;
import javax.swing.JButton;
public interface Switchable {
public void toSwitch(List<JButton> gameFieldButtons, int... indizes);
public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps);
}
import java.awt.Color;
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;
public class SwitchOn implements Switchable {
@Override
public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
switchOn(gameFieldButtons, indizes);
}
@Override
public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
switchOn(selectedButton, gameFieldButtons, steps);
}
private void switchOn(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
for (int i = 0; i < steps.length; i++) {
int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
enable(indexFromButtonToBeDisabled, gameFieldButtons);
}
}
private void switchOn(List<JButton> gameFieldButtons, int... indizes) {
for (int i = 0; i < indizes.length; i++) {
enable(indizes[i], gameFieldButtons);
}
}
private void enable(int buttonIndex, List<JButton> gameFieldButtons) {
JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
buttonToBeDisabled.setEnabled(true);
buttonToBeDisabled.setBorder(new LineBorder(Color.red, 1, false));
}
}
import java.awt.Color;
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;
public class SwitchOff implements Switchable {
@Override
public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
switchOff(gameFieldButtons, indizes);
}
@Override
public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
switchOff(selectedButton, gameFieldButtons, steps);
}
private void switchOff(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
for (int i = 0; i < steps.length; i++) {
int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
int valueFromButtonToBeDisabled = steps[i] + valueFromSelectedButton;
int indexFromButtonToBeDisabled = valueFromButtonToBeDisabled - 1;// because gameFieldButtons starts by 0
disable(indexFromButtonToBeDisabled, gameFieldButtons);
}
}
private void switchOff(List<JButton> gameFieldButtons, int... indizes) {
for (int i = 0; i < indizes.length; i++) {
disable(indizes[i], gameFieldButtons);
}
}
private void disable(int buttonIndex, List<JButton> gameFieldButtons) {
JButton buttonToBeDisabled = gameFieldButtons.get(buttonIndex);
buttonToBeDisabled.setEnabled(false);
buttonToBeDisabled.setBorder(new LineBorder(Color.lightGray, 1, false));
}
}
Don't Repeat Yourself (DRY)
Well as we can see this is a lot better but we can still see a lot of dupplicated code inside of it. The only difference in the implementation from
SwitchOn or
SwitchOff is the
setEnable(true) or
setEnable(false) in the methods
enable(...) or
disable(...) and the LineBorder of both. Let's do one more elegant change to it like this:
import java.util.List;
import javax.swing.JButton;
import javax.swing.border.LineBorder;
public abstract class SwitchAbstract implements Switchable {
@Override
public void toSwitch(List<JButton> gameFieldButtons, int... indizes) {
switchWithIndex(gameFieldButtons, indizes);
}
@Override
public void toSwitch(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
switchWithSteps(selectedButton, gameFieldButtons, steps);
}
private void switchWithIndex(List<JButton> gameFieldButtons, int... indizes) {
for (int i = 0; i < indizes.length; i++) {
turnSwitchTo(indizes[i], gameFieldButtons);
}
}
private void switchWithSteps(JButton selectedButton, List<JButton> gameFieldButtons, int... steps) {
for (int i = 0; i < steps.length; i++) {
int valueFromSelectedButton = Integer.valueOf(selectedButton.getText());
int valueFromButtonToBeSwitched = steps[i] + valueFromSelectedButton;
int indexFromButtonToBeSwitched = valueFromButtonToBeSwitched - 1;// because gameFieldButtons starts by 0
turnSwitchTo(indexFromButtonToBeSwitched, gameFieldButtons);
}
}
private void turnSwitchTo(int buttonIndex, List<JButton> gameFieldButtons) {
JButton buttonToSwitch = gameFieldButtons.get(buttonIndex);
buttonToSwitch.setEnabled(switchTo());
buttonToSwitch.setBorder(howShallLineBorderLooksLike());
}
protected abstract boolean switchTo();
protected abstract LineBorder howShallLineBorderLooksLike();
}
public class SwitchOn extends SwitchAbstract {
@Override
protected boolean switchTo() {
return Boolean.TRUE;
}
@Override
protected LineBorder howShallLineBorderLooksLike() {
return new LineBorder(Color.red, 1, false);
}
}
public class SwitchOff extends SwitchAbstract {
@Override
protected boolean switchTo() {
return Boolean.FALSE;
}
@Override
protected LineBorder howShallLineBorderLooksLike() {
return new LineBorder(Color.lightGray, 1, false);
}
}
Why did i name the class SwitchAbstract and not AbstractSwitch?
Well maybe you did not noticed or it is not so obvious at first sight, but i has a good reason. the reason is in the way i'm able to search in my IDE. Naming my classes this way alls "Switch..." classes will be presented as a list in a very nice way while looking for the keyword "switch" in Eclipse.
Results in facts:
When talking about clean code i ofen hear the argumet: It makes my code a lot more complex and generate a lot of useless code. Well let's see if this is true. I have more classes. For this case it must be automatically more complex and of course it must be have generated more lines of code right?
Old version: Lines of code: 96 Lines of code
New version: Lines of code: 73 Lines of code
Old class and interface names: Naming was not that simple understandable.
New class and interface names: Clear, does not implies more then one responsabilities
Old class implementation: Costs a lot and had ugly switch-cases and enums in it.
New class implemention: No effort, no enums, no ungly switche-cases
😱👇 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO 😱👇
Be sure to read, it will change your life!
This book is a must read - it will put you in another level! (Expert)
Write cleaner code and stand out!
This book is very practical, straightforward and to the point! Worth every penny!
Needless to say, these are top right?
😱👆 PROMOTIONAL DISCOUNT: BOOKS AND IPODS PRO 😱👆
