diff --git a/_learning_hours/refactoring/split_loop.md b/_learning_hours/refactoring/split_loop.md index 363e14e..893e69a 100644 --- a/_learning_hours/refactoring/split_loop.md +++ b/_learning_hours/refactoring/split_loop.md @@ -8,7 +8,7 @@ difficulty: 2 author: emilybache --- -# Split Loop (simpler version) +# Split Loop Merge conflicts are a major cause of bugs - people get in each other's way when they are editing the same lines of code because the same section of code does more than one thing. It would be better to split up the logic according to each responsibility. Today we’re going to look at how to divide it up safely. @@ -45,7 +45,7 @@ Explain that the reason you wanted them to classify the code like this is becaus For each type of change, the impacted lines of code are all over the place. What we'd be doing is a form of [Shotgun Surgery]({% link _code_smells/shotgun_surgery.md %}). This introduces a danger of merge conflicts if the changes are happening at the same time, and also the danger you'll miss somewhere that needs updating. We'd like to refactor the code before we begin either change. ## Demo: Split loop -In Theatrical players show that you need to do several refactorings in order to gather the presentation logic together and separate it from the calculation logic. Show how to do that safely, including [Split Loop]({% link _refactorings/split_loop.md %}). Use the simpler form where you inline all the local variables. +In Theatrical players show that you need to do several refactorings in order to gather the presentation logic together and separate it from the calculation logic. Show how to do that safely, including [Split Loop]({% link _refactorings/split_loop.md %}). Keep it simple - don't do a [Split Phase]({% link _refactorings/split_phase.md %}), simply inline all the local variables so they are calculated when needed. ## Concrete Practice: Split loop in Theatrical Players and Expense Report In pairs, do the same exercise in Theatrical Players that you just demonstrated. When they have done that one, they could try [Expense Report](https://github.com/emilybache/ExpenseReport-Refactoring-Kata) which is very similar. diff --git a/_learning_hours/refactoring/split_loop_advanced.md b/_learning_hours/refactoring/split_phase.md similarity index 74% rename from _learning_hours/refactoring/split_loop_advanced.md rename to _learning_hours/refactoring/split_phase.md index 4677e35..1e7bcfc 100644 --- a/_learning_hours/refactoring/split_loop_advanced.md +++ b/_learning_hours/refactoring/split_phase.md @@ -1,7 +1,7 @@ --- theme: refactoring -title: Split Loop (Advanced) -name: split_loop_advanced +title: Split Phase +name: split_phase code_smell: divergent_change kata: theatrical_players difficulty: 3 @@ -9,19 +9,19 @@ author: emilybache --- -# Split Loop (more difficult version) +# Split Phase -Divergent Change is a code smell that describes when the same section of code needs to change because of several different reasons. It would be better to divide up the responsibilities. This learning hour is similar to the one on [Split Loop]({% link _learning_hours/refactoring/split_loop.md %}) but is intended to show the more advanced version of the split loop refactoring. +Divergent Change is a code smell that describes when the same section of code needs to change because of several different reasons. It would be better to divide up the responsibilities. This learning hour is similar to the one on [Split Loop]({% link _learning_hours/refactoring/split_loop.md %}) but is intended to show how to combine Split Phase with Split Loop. # Learning Goals -* Use "Split Loop" to separate concerns into different parts of the code when there are awkward local variables +* Use "Split Phase" to separate concerns into different sequential steps # Session Outline * 10 min connect: Classify the code * 10 min concept: Make the Change Easy -* 5 min demo: Split Loop (advanced) -* 35 min do: Split loop in Theatrical Players and Office Cleaning Robot +* 5 min demo: Split Phase +* 35 min do: Split Phase in Theatrical Players and Office Cleaning Robot * 5 min reflect: Explain the main idea @@ -44,13 +44,13 @@ Explain that the reason you wanted them to classify the code like this is becaus Looking at the top level loop in 'parseInput', this code suffers from [Divergent Change]({% link _code_smells/divergent_change.md %}). This loop does two things that don't need to be together. We'd like to refactor the code before we begin developing new features. As Kent Beck says 'make the change easy (warning: this may be hard), then make the easy change'. -## Demo: Split loop -In either [Theatrical Players](https://github.com/emilybache/Theatrical-Players-Refactoring-Kata) or [Office Cleaning Robot](https://github.com/sammancoaching/OfficeCleaningRobot-Refactoring-Kata) (version 1) show that you need to do several refactorings in order to separate the different kinds of logic. Show how to do that safely, including [Split Loop]({% link _refactorings/split_loop.md %}). Use the more advanced form where you create a new class to hold the local variables. +## Demo: Split Phase +In either [Theatrical Players](https://github.com/emilybache/Theatrical-Players-Refactoring-Kata) or [Office Cleaning Robot](https://github.com/sammancoaching/OfficeCleaningRobot-Refactoring-Kata) (version 1) show that you need to do several refactorings in order to separate the different kinds of logic. Show how to do that safely, including [Split Phase]({% link _refactorings/split_phase.md %}) and also [Split Loop]({% link _refactorings/split_loop.md %}). -## Concrete Practice: Split loop in Theatrical Players and Office Cleaning Robot -In pairs, do the same split loop in both [Office Cleaning Robot](https://github.com/sammancoaching/OfficeCleaningRobot-Refactoring-Kata) (version 1) and Theatrical Players. +## Concrete Practice: Split Phase in Theatrical Players and Office Cleaning Robot +In pairs, do the same split phase and split loop refactorings in both [Office Cleaning Robot](https://github.com/sammancoaching/OfficeCleaningRobot-Refactoring-Kata) (version 1) and Theatrical Players. ## Conclusions: Explain the main idea -[Explain the main idea]({% link _activities/conclusions/explain_main_idea.md %}) If someone asked you when and how to do 'split loop', what would you tell them? Everyone note down their ideas individually. +[Explain the main idea]({% link _activities/conclusions/explain_main_idea.md %}) If someone asked you when and how to do 'split phase', what would you tell them? Everyone note down their ideas individually. diff --git a/_refactorings/split_loop.md b/_refactorings/split_loop.md index 6280c5a..79df6b7 100644 --- a/_refactorings/split_loop.md +++ b/_refactorings/split_loop.md @@ -3,63 +3,29 @@ layout: refactoring title: Split Loop source: Emily Bache source_url: https://refactoring.com/catalog/splitLoop.html -code_smells: loops, divergent_change, shotgun_surgery -learning_hours: split_loop, split_loop_advanced +code_smells: loops,divergent_change,shotgun_surgery +learning_hours: split_loop --- # Split Loop This is one of the refactorings in Martin Fowler's book. Often this refactoring opens up for a lot of other useful changes. ## Examine -A loop is of itself a code smell, but even more so when it does more than one thing, which implies Divergent Change. +A loop is of itself a code smell, but even more so when it does more than one thing, which implies Diverent Change. ## Prepare Identify the loop you want to split, and in particular which lines within it that hang together and need to be separated from the others. You may need to do some 'introduce variable' refactorings and 'slide statement' refactorings to gather all the relevant parts of the loop together. -You may discover there are local variables that you will eventually need in both halves of the split loop. There are two main ways to deal with these local variables: - -* Re-calculate the variable every time you need it. -* Create a new class and collection for the second loop. - -The first option is simpler, but in many cases re-calculation may be costly or impossible. The second option avoids re-calculating the variables every time you need them but is more difficult to do. The mechanics of both are outlined below. - ## Implement -### Simple version -This version is for when you have already dealt with any local variables that will be needed in both halves of the loop: - * Tests all passing * Select the whole loop and duplicate it. * In the second copy of the loop, delete _everything except_ the lines you want to split. * In the first copy of the loop, delete _only_ the lines you want to split. * Tests all passing. -### Advanced version -This version is for when you have local variables you will need to use in both halves: - -* Tests all passing -* Identify all local variables needed by the part of the loop you want to split out. -* Create a new class 'LocalData' with fields for all these local variables and a constructor that initializes them. -* Create an instance of the new class called 'localData'. -* Tests all passing ('localData' is not yet used). -* Change the rest of the loop to use 'localData' members instead of the local variables directly. -* Tests all passing. -* Create a new collection before the loop to hold all the instances of 'localData', and in the loop, append the 'localData' instances to the collection. -* Tests all passing ('localData' collection is not used). - -Now split the loop: - -* Select the whole loop and duplicate it. -* In the second copy of the loop, delete everything except the lines you want to split. -* In the first copy of the loop, delete _only_ the lines you want to split. - -At this point the code is broken because there are references to 'localData' in the second loop. - -* Change the second loop to instead use your new collection of 'localData' instances instead of what it was looping over before. -* Tests all passing - ## Clear -Double-check the remaining logic in the original loop makes sense and that you didn't delete too much by mistake. Adjust names for loop variables - something better than 'localData'. You may want to split the original loop again to separate calculation of 'localData' from the rest of the logic. +Double-check the remaining logic in the original loop makes sense and that you didn't delete too much by mistake. ## Follow up Often you now want to extract a method for the new loop, or turn it into a pipeline. diff --git a/_refactorings/split_phase.md b/_refactorings/split_phase.md new file mode 100644 index 0000000..8996ae0 --- /dev/null +++ b/_refactorings/split_phase.md @@ -0,0 +1,53 @@ +--- +layout: refactoring +title: Split Phase +source: Emily Bache +source_url: https://refactoring.com/catalog/splitPhase.html +code_smells: loops, divergent_change, shotgun_surgery +learning_hours: split_phase +--- + +# Split Phase +This is one of the refactorings in Martin Fowler's book. Often this refactoring opens up for a lot of other useful changes. + +## Examine +Look for one section of code that is doing two separate things - ie you have [Divergent Change]({% link _code_smells/divergent_change.md %}). + +## Prepare +Identify the two concerns you want to split. You may need to do some 'introduce variable' refactorings and 'slide statement' refactorings to gather all the relevant parts together. Aim for two sections of code that each do one thing. There will probably be a number of variables which are calculated in the first section and used in the second section. + +## Implement + +* Tests all passing +* Identify all local variables needed by the second section that are calculated in the first. One way to do this is use a tool to 'extract method' on the second section - the argument list to the new method contains these. +* Create a new class 'LocalData' with fields for all these local variables and a constructor that initializes them. +* Create an instance of the new class called 'localData' between the two sections of code. +* Tests all passing ('localData' is not yet used). +* Change the second section of code to use 'localData' members instead of the local variables or individual method arguments. +* Tests all passing. +* Extract method on the second section (if you didn't already do this) - argument is 'localData'. +* Extract method on the first section - return value is 'localData'. +* Tests all passing. + +## Clear +Adjust names for variables - something better than 'localData'. + +## Follow up + +If the two sections of code are in the same loop, you should now split it. There is a useful trick for making this easier: + +* Create a new collection before the loop to hold all the instances of 'localData', and in the loop, append the 'localData' instances to the collection. +* Tests all passing ('localData' collection is not used). + +Now [split the loop]({% link _refactorings/split_loop.md %}): + +* Select the whole loop and duplicate it. +* In the second copy of the loop, delete everything except the lines you want to split. +* In the first copy of the loop, delete _only_ the lines you want to split. + +At this point the code is broken because there are references to 'localData' in the second loop. + +* Change the second loop to instead use your new collection of 'localData' instances instead of what it was looping over before. +* Tests all passing + +Often you now want to turn both loops into pipelines, and/or start making LocalData into a proper domain class and moving methods to it.