There are a lot of applications that are very difficult to maintain. The original writers would not always agree about that statement, but others will see their code as an unmanageable mess.
I will not blame the original writers of those applications. A lot of them started with software development as their hobby or out of interest. During time they learned some "best practises" by reading tutorials about how to write desktop applications using forms (like in Visual Basic or Delphi). These tutorials tell you that in the event handlers you put the code that should be run when somebody presses a button or when an item is selected. There are even components like timers that could let you do stuff at specific times / intervals.
For tutorials this is OK and I did it as well, but I realised that it is plain stupid to do so. The problem is that you get large chunks of code within event handlers and forms. At some point it will be difficult to know where some piece of code can be found. Is it on FormA, FormB or (even worse) on both?
In the rest of this post I want to describe some small steps to re-factor to easier to maintain and understand code. As an example I use a simple application to manage data from media files. There is a form that add/edit music, it has a save button and a close button.
Get code out of event handlers.
The first step is to get event handlers as small as possible. Put all the code that is currently in the event handler into a method and call that method in the event handler. An example:
In the form for adding and editing music information we have a lot of code in the OnSaveClicked event handler. There is code to see whether we are adding or editing the music, there is code to fill the data set (or the sql query, or whatever we are using for persistence) and there is some validation code. That would be at least 15 to 30 rows of code. (depending on how much information we can fill in). By moving all this code to a separate method we can easily call this same method from the OnClosed event handler to let the user save the changes when closing the form.
Split code that is interacting with the controls from other code.
In the method to save the music information we are still asking the information from every input control. So let say we have a textbox to fill in the year of when the music is written, we would call txtYear.Text to get the information. We will probably do this at least two times (one for the validation, because this is a required field and two for saving the value). When we want to change to control to a DateTime control, we have change the code for validation and the code for saving the value which could lead to unexpected behaviour (a.k.a. bugs).
A better way would be to have a method that accepts parameters (one for every field that should be saved). You will then get the following code:
public void OnSavedEvent(object sender, EventArg e)
{
SaveMusic();
}
private void SaveMusic()
{
string name = txtName.Text;
// int year = Int32.ParseToInt(txtYear.Text); // we used a textbox but now we have a special control called YearPicker which gives back an integer.
int year = yearPicker.Year;
string composer = txtComposer.Text;
SaveMusic(name, year, composer);
}
private void SaveMusic(string name, int year, string composer)
{
// do a lot of stuff with name, year and composer
}
This way we can easily change a control without worrying about whether something goes wrong with saving the data.
Separate concerns.
To be honest, I don't like to much knowledge within a form. A form is there to display data and to handle input and a little but for orchestrate the application flow (more on this in a later post).
Doing stuff with data (and getting the data) could better be done in a separate class. So we can create a class called MusicDataService, which has the SaveMusic(string name, int year, string composer) method. It will also get some methods to load the data.
public struct Music
{
public string Name;
public int Year;
public string Composer;
}
public class MusicDataService
{
public void SaveMusic(Music music) {/* lot of stuff to be done... */ }
public Music GetMusicWithName(string name) {/* lot of stuff to be done... */ }
}
public class MusicAddView()
{
private MusicDataService _service = new MusicDataService();
public void OnSavedEvent(object sender, EventArg e)
{
SaveMusic();
}
private void SaveMusic()
{
Music music = new Music();
music.name = txtName.Text;
music.year = yearPicker.Year;
music.composer = txtComposer.Text;
_service.SaveMusic(music);
}
}
Now that we have the knowledge of saving music separated from it's form, we can use the same code at multiple places if we want.
Smaller methods.
As I said the SaveMusic method in the MusicDataService contains a lot of logic. It validates the input and it makes sure that it gets persisted. Since validation on it's own can contain a lot of code, it is recommended to put this in a separate method as well. This gives us an other advantage. We can now validate the Music structure multiple times even when we don't want to save. When editing the object we can directly validate the input and gave the user feedback.
The real implementations of validating and saving data I will not show, since this is very specific for each applications and there are just to much ways of doing it (and frameworks that could be used). This said it is probably a good idea to make that code pluggable, so we can swap the implementation if we want to. (how to do this could be another post.. you can also use google)
Conclusion.
In this post I tried to show that using some easy steps it is not to difficult to make your program easier to maintain, easier to test and easier to understand for new software developers. And this all leads to a more solid application.
