Skip to content

Commit

Permalink
split loop (advanced) should be instead split phase
Browse files Browse the repository at this point in the history
  • Loading branch information
emilybache committed Jan 13, 2025
1 parent c5e469f commit 298a2e2
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 52 deletions.
4 changes: 2 additions & 2 deletions _learning_hours/refactoring/split_loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
---
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
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


Expand All @@ -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.


42 changes: 4 additions & 38 deletions _refactorings/split_loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 53 additions & 0 deletions _refactorings/split_phase.md
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 298a2e2

Please sign in to comment.