This article is a revised version. It was previously published atย refactoring-legacy-code.netย published.
This is a series of articles. You can find the other articles here:
- Simple refactorings - Part 1
- Simple refactorings - Part 2
- Simple refactorings - Part 3
- Simple refactorings - Part 5
Reduce dependencies - Extract Method and Introduce Parameter
The following excerpt from an alarm clock application shows the code that is executed when the Start button is pressed:
start.click += (s, e) => { if (txtWhen.Text == "") { txtWann.Text = DateTime.Now.ToLongTimeString(); } var alarm_time = DateTime.Parse(txtWann.Text); Alarm_start(alarm_time); };
In the first step, a Extract Method to make the code easier to understand. The additional method makes it possible for the reader to understand what is happening with the Click event should happen:
start.click += (s, e) => { var alarm_time = alarm_time_means(); Alarm_start(alarm_time); };
The extracted method is as follows:
private DateTime Wake_up_time() { if (txtWhen.Text == "") { txtWann.Text = DateTime.Now.ToLongTimeString(); } var alarm time = DateTime.Parse(txtWann.Text); return wake-up time; }
In this method, I would like to use a Introduce parameters Refactoring the dependency of the method on the control element txtWhen from the user interface. To do this, I select the control txtWhen including the property Textso here "txtWhen.text" and then execute with Introduce parameters a method parameter. The IDE suggests replacing the access with the parameter in two places. Only two of the three usage locations are suggested, as the third usage is write-only. The result of the refactoring is shown in the following listing:
private DateTime Wake_time_mean(string wake_time_as_text) { if (wake_time_as_text == "") { txtWann.Text = DateTime.Now.ToLongTimeString(); } var alarm_time = DateTime.Parse(alarm_time_as_text); return wakeuptime; }
I am not satisfied with this result, as the control is still being accessed. Furthermore, the semantics have now changed! Within the condition that checks whether the text is empty, the current time is set in the control. However, this value is then no longer used by the following DateTime.Parse Call taken over. I therefore undo the refactoring. The problem here is that two aspects are subtly mixed:
- Parses the alarm time from the user input.
- Set the text box to the current time as the default value if no input is available.
Before further refactorings, I therefore first separate the two aspects. The first step is a Extract Methodwhich I use to remove the optional assignment of the default value from the method:
public DateTime Set_wake_up_time() { Default_set(); return DateTime.Parse(txtWann.Text); } private void Default_setzen() { if (txtWann.Text == "") { txtWann.Text = DateTime.Now.ToLongTimeString(); } }
I then remove the call from Set_default and move it to the call location of Set_wake_up_time.
start.click += (s, e) => { Default_set(); var alarm_time = alarm_time_means(); Alarm_start(alarm_time); };
Now I can work both in Set_default as well as in Set_wake_up_time with Introduce parameters ensure that it is clear on which inputs the methods work:
public DateTime Wake_up_time(string text) { return DateTime.Parse(text); } private void Default_set(TextBox textBox) { if (textBox.Text == "") { textBox.Text = DateTime.Now.ToLongTimeString(); } }
In Set_wake_up_time I have txtWhen.text is replaced by the parameter. In Set_default I had to delete the whole control due to the write access. txtWhen with the parameter. The calls now look like this:
start.click += (s, e) => { Default_set(txtWhen); var alarm_time = alarm_time_set(txtWann.Text); Alarm_start(alarm_time); };
I am satisfied with this result. Now the process is clearly recognizable: first a default value is set on the control. Then the alarm time is determined from the user input and the alarm clock is started with this alarm time. What remains is the dependency on the current time, represented by DateTime.Now.
Conclusion
Use Extract Method and Introduce parametersto Separate aspects and Reduce dependencies. Please note, however, that these refactorings are no longer possible solely by using a refactoring tool. To ensure that you do not change the semantics of the program, you need a Safety net from Version controlautomated and manual Tests and Code Reviews.
Separate aspects - Move To Another Type
Different aspects are often mixed within a class. Sometimes the aspects are already distributed across several methods, but sometimes they are mixed within the methods. As long as the aspects are still mixed within a method, you can try to use the Extract Method Refactoring code areas from the method into additional methods in order to separate the aspects at method level. The methods can then be assigned to different classes in order to ensure aspect separation at class level. The Move To Another Type refactoring. In the following example, a CSV viewer was created within a single class, in this case the class Program, realized. The listing shows an excerpt:
class Program { static void Main(string[] args) { var rawLines = File.ReadAllLines(args[0]); var pageLen = 5; if (args.Length > 1) pageLen = int.Parse(args[1]); var pageLines = rawLines.Take(pageLen + 1); var iFirstLineOfLastPage = 1; while (true) { var records = pageLines.Select(l => Convert_line_to_record_fields(l, ",")); ... } } private static string[] Convert_line_to_record_fields(string line, string delimiter) { return Convert_line_to_record_fields(line, delimiter, new List()).ToArray(); } private static List Convert_line_to_record_fields(string line, string delimiter, List fields) { if (line == "") return fields; if (line.StartsWith("\"")) { ... } return Convert_line_to_record_fields(line, delimiter, fields); } }
The aspects have already been partially separated at the method level. The topic Splitting CSV rows are combined into two methods. These two methods should now be moved from the Program class to a separate class in order to separate the aspects at class level.
Before extracting the two methods, their visibility must be checked by private to public can be changed. This intervention does not usually involve any risk. Then select one of the methods to be moved to a separate class and start the Move To Another Type Refactoring. Depending on the refactoring tool or IDE used, a query or selection of the class to which the method is to be moved is then made. You are also asked which methods are to be moved. Here it is important to select all methods that belong to the same aspect. In the example, I store the two variants of the method Convert_line_to_record_fields to the CSV class. Using a refactoring tool, such as ReSharper, ensures that the method calls are adapted so that they now refer to the new class.
var records = pageLines.Select(l => CSV.Convert_line_to_record_fields(l, ",")); static internal class CSV { public static string[] Convert_line_to_record_fields(string line, string delimiter) { return Convert_line_to_record_fields(line, delimiter, new List()).ToArray(); } public static List Convert_line_to_record_fields(string line, string delimiter, List fields) { ... } }
Conclusion
Use the Move Method To Another Type Refactoring to Aspects at the class level separate. If the aspects are still mixed within methods, first apply the Extract Method Refactoring to separate the aspects at the level of methods.