On an application I’ve been working on I have a function, connecting to a virtual machine, that can be performed two ways. The user can either select a machine from a list and click the “Connect” button. Or can double click on the machine in the list. Currently they are two separate event handlers because they function slightly differently. If a user is already connected, clicking the button will disconnect them, and double clicking will bring the window back up.
///edit
The logic is essentially
button
if(connected)
Disconnect()
else
ConnectAndShowWindow()
List
ConnectAndShowWindow()
My question is should I keep the methods separate or combine the two methods even though they are slightly different and then, for the disconnecting/reconnecting switch them based on the sender?
2
Your features are:
- Connect
- Disconnect
- Bring Window Forward
The logic of the “Connect” button is:
if (connected)
Disconnect
else
Connect
The logic of the machine double-click is:
if (connected)
Bring Window Forward
else
Connect
To me, these are quite separate handlers and I would leave them so. They should of course call the common Connect method.
2
An approach that I’ve used in that type of situation is to refactor the two methods into a single private method with a parameter to indicate which of the paths should be taken.
public void buttonEventHandler() {
connectHandler(ConnectEvent.BUTTON);
}
public void DoubleClickEventHandler() {
connectHandler(ConnectEvent.DOUBLE_CLICK);
}
private void connectHandler(ConnectEvent eventOption) {
// conditional connection logic
}
This keeps the interface simple and allows the connect code to be shared in place.
3
So, what you’re saying is that the software acts one way when the software is in one state and another thing when the software is in another state. Well, welcome to the State Pattern.
The short story is that you want two classes, each implementing the same interface. the interface should have two methods — OnMachineConnectSelected and OnMachineDoubleClick.
The first class is called DisconnectedState, the second class is called ConnectedState.
- DisconnectedState performs Connect and ShowWindow from OnMachineConnectSelected.
- DisconnectedState performs Connect from OnMachineDoubleClick.
- ConnectedState performs ShowWindow from both OnMachineConnectSelected and OnMachineDoubleClick.
(It’s hard to tell without more specifics, but you can and probably should have Connect and ShowWindow as methods on your form or other application object and pass a reference, probably via another interface, into the constructor of your state class.)
Now your application simply needs an object on which it can call those methods, as the actions occur, and you can switch that object between an instance of DisconnectedState and ConnectedState as the actual state of your application changes.
See what this does for you?
First, it models your application very well. Look at the bullet-points above. You wouldn’t have to rearrange the words much to explain what was going on to a user or non-technical manager. You are separating out the concepts of state (software/hardware decisions) and action (user decisions) neatly.
Second, it allows for extensibility. When you realise that you need to act very differently, while connecting and disconnecting, you can simply add two new state classes. Or if you need to add a new action that depends on the state, you can simply add a new method to all your state objects.
Third, it is more maintainable. When a business person says “Hey, currently, when the application is Disconnected and you Select Connect on a machine, it Connects and Shows the Window; but we want it to simply connect,” you know EXACTLY where that logic exists in the code and can edit it easily, safe in the knowledge that you aren’t breaking any of the other conditions.
Finally, it’s more unit-testable. There is only one path through each method. Pass a mock form into your class constructor, call the OnX method and make sure it performs the right actions. And, on the form, you can test that your action calls the right method, no matter what state it’s in.