I have a class with many switch statements in it. The code looks very ugly to me but I can't figure out how to fix it. If somebody could suggest a design pattern or a trick that cleans this up that would be great. Below is a abbreviated version of the Class with some annotations for context.
/**
* Slides a desktop Notification from the edge of the screen into the desktop by some margin.
*/
public class SlideManager extends NotificationManager {
// Location enum provided below for reference:
/*public enum Location {
NORTH, NORTHEAST, EAST, SOUTHEAST, SOUTH, SOUTHWEST, WEST, NORTHWEST
}*/
private Location m_loc; // the corner / edge of the screen where the Notification should appear
// A Screen abstracts away the padding logic.
// The idea was that you would just give it a padding and I could reuse
// it later in all my NotificationManagers to give me the x and y for a Notification Location later.
private Screen m_standardScreen;
private Screen m_noPaddingScreen;
// the direction that the Notification should slide in from.
// For instance, if we had our Notification show up on the center east side of the screen,
// it should slide in from the edge towards the west until it was a suitable margin away from the edge of the screen.
private SlideDirection m_direction;
private double m_slideSpeed;
// this is a flag to signal when the user has overridden the default SlideDirection calculation.
// If the user constructs the SlideManager without calling setSlideDirection later,
// this will be false. However, if the user does setSlideDirection
// we want to avoid changing that automatically if the user later calls setLocation.
private boolean m_overwrite;
public enum SlideDirection {
NORTH, SOUTH, EAST, WEST
}
{
m_standardScreen = Screen.standard();
m_noPaddingScreen = Screen.withPadding(0);
m_slideSpeed = 300;
m_overwrite = false;
}
public SlideManager() {
m_loc = Location.NORTHEAST;
recalculateSlideDirection();
}
public SlideManager(Location loc) {
m_loc = loc;
recalculateSlideDirection();
}
/**
* Sets the location where the Notifications show up.
* If the user has not explicitly given a SlideDirection, this will also recalculate the SlideDirection.
* For example, if the user moves the Notification spawn location from East to West
* we want to change the SlideDirection from West to East, respectively.
*
* @param loc
*/
public void setLocation(Location loc) {
m_loc = loc;
if (!m_overwrite)
recalculateSlideDirection();
}
/**
* Sets the direction that the Notification should slide in from.
*
* @param slide
*/
public void setSlideDirection(SlideDirection slide) {
m_direction = slide;
m_overwrite = true;
}
private void recalculateSlideDirection() {
switch (m_loc) {
// The tricky part is when the user wants the Notification to appear in a corner of the screen.
// If this weren't the case, it would make no sense for a user to set
// his own SlideDirection because the SlideDirection for Location.
// EAST would always be West, Location.NORTH would always be SOUTH, etc.
// But for the (literal) corner cases I want to somehow give a way for the user to have a preference.
// For example, in the top right corner the Notification could either slide in from the right or from the top.
// By default I choose the top, but I want the user to be able to change this and also have that choice remembered if the Location changes.
case NORTHWEST:
m_direction = SlideDirection.SOUTH;
break;
case NORTH:
m_direction = SlideDirection.SOUTH;
break;
case NORTHEAST:
m_direction = SlideDirection.SOUTH;
break;
case EAST:
m_direction = SlideDirection.WEST;
break;
case SOUTHEAST:
m_direction = SlideDirection.NORTH;
break;
case SOUTH:
m_direction = SlideDirection.NORTH;
break;
case SOUTHWEST:
m_direction = SlideDirection.NORTH;
break;
case WEST:
m_direction = SlideDirection.EAST;
break;
}
}
/*
When a Notification is added it should slide in from the edge towards an area slightly off the edge.
*/
@Override
protected void notificationAdded(Notification note, Time time) {
int noPaddingX = m_noPaddingScreen.getX(m_loc, note);
int noPaddingY = m_noPaddingScreen.getY(m_loc, note);
int standardX = m_standardScreen.getX(m_loc, note);
int standardY = m_standardScreen.getY(m_loc, note);
Slider slider = null;
double frequency = 50;
double slideDelta = m_slideSpeed / frequency;
// How would I abstract this?
switch (m_direction) {
case NORTH: {
note.setLocation(standardX, noPaddingY);
slider = new Slider(note, m_direction, 0, -slideDelta, standardX, standardY);
}
break;
case SOUTH: {
note.setLocation(standardX, noPaddingY);
slider = new Slider(note, m_direction, 0, slideDelta, standardX, standardY);
}
break;
case EAST: {
note.setLocation(noPaddingX, standardY);
slider = new Slider(note, m_direction, slideDelta, 0, standardX, standardY);
}
break;
case WEST:
note.setLocation(noPaddingX, standardY);
slider = new Slider(note, m_direction, -slideDelta, 0, standardX, standardY);
break;
}
Timer timer = new Timer((int) frequency, slider);
timer.start();
note.show();
}
/*Slides a Notification from its current location to a desired location with fixed deltaX's and deltaY's. It needs the SlideDirection to know which end values to check for stopping.*/
public class Slider implements ActionListener {
private Notification m_note;
private SlideDirection m_dir;
private double m_deltaX;
private double m_deltaY;
private double m_stopX;
private double m_stopY;
private double m_x;
private double m_y;
public Slider(Notification note, SlideDirection dir, double deltaX, double deltaY, double stopX, double stopY) {
m_note = note;
m_dir = dir;
m_deltaX = deltaX;
m_deltaY = deltaY;
m_stopX = stopX;
m_stopY = stopY;
m_x = note.getX();
m_y = note.getY();
}
@Override
public void actionPerformed(ActionEvent e) {
m_x += m_deltaX;
m_y += m_deltaY;
// another ugly heap of conditionals. Is there a way to abstract away the stopping logic?
if (m_dir == SlideDirection.SOUTH) {
if (m_y >= m_stopY) {
m_y = m_stopY;
((Timer) e.getSource()).stop();
}
} else if (m_dir == SlideDirection.NORTH) {
if (m_y <= m_stopY) {
m_y = m_stopY;
((Timer) e.getSource()).stop();
}
} else if (m_dir == SlideDirection.EAST) {
if (m_x >= m_stopX) {
m_x = m_stopX;
((Timer) e.getSource()).stop();
}
} else if (m_dir == SlideDirection.WEST) {
if (m_x <= m_stopX) {
m_x = m_stopX;
((Timer) e.getSource()).stop();
}
}
m_note.setLocation((int) (m_x), (int) (m_y));
}
}
}
Obviously, having two switchs and a conditional chain means something is wrong. My initial thought was to make some kind of a SlideVector class where I could give it a SlideDirection and it would calculate the translations and handle the end cases. What are the pros/cons of this approach? Is a HashMap of any use in this case? Are there any other ways I can approach this problem?
Look at the strategy pattern. You can move all your switch statements into a different class. Your class will have an instance of this translation class that will have a method to which you feed the state (all the information) and it will create the output for you.
In recalculateSlideDirection you can use fall through to save on lines (although ehh, it's not going to save you a lot).
Another option for notificationAdded is to define an interface and create a hashmap that will map the m_direction to an implemention of that interface. So you basically will just have a HashMap< SlideDirection, IYourInterface> yourMap... And so you will basically call yourMap.get(n_direction).action(); and this will replace your switch statement.