Refactoring In Tdd The Missing Part

Debugging
Sucks

Testing
Rocks

Bugfixing
Cost

Bugs

• Cost of bugfix = Number of bugs x Cost per fix, but...

• How many bugs do you plan to have?

• How hard do you think they are to fix?

Bugfixing
Tests
Cost

Bugs/Features

• Cost of tests = Number and complexity of features

• How many features do you plan to have?

• How complex they are?

Fixture

all the things we need to
have in place in order
to run a test and expect
a particular outcome

Fixture Setup
Fixture

Steps

Fixture Setup

SUT

System Under Test
is whatever thing we
are testing

Exercise SUT
Fixture

Steps
SUT
Fixture Setup

Exercise SUT

Verify Result
Fixture

Steps
SUT
Fixture Setup

Exercise SUT

Verify Result

“Don't call us, we'll call you”

Fixture Teardown
Fixture

Steps
SUT
Fixture Setup

Exercise SUT

Verify Result

Fixture Teardown

a test is a good one if...
• Is really automatic
• Should be easy to invoke one or more tests
• Must determine for itself whether it passed
or failed
• Test everything that’s likely to break
• Must be independent from the environment
and each other test
• Should be repeatable, could be run over and
over again, in any order and produce the same
results
• The code is clean as the production code

the kind of test is not
determined by the used tool

Unit tests
A test is not a unit test if:
1. It talks to a database
2. It communicates across the network
3. It touches the file system
4. You have to do things to your environment
to run it (eg, change config files)
Tests that do this are integration tests

Michael Feathers

public void marriageIsSimmetric() {
Customer alice = new Customer("alice");
Customer bob = new Customer("bob");

bob.marry(alice);

assertTrue(bob.isMarriedTo(alice));
assertTrue(alice.isMarriedTo(bob));
}

Simple Unit Test


bob.marry(alice);

}

Fixture Setup


bob.marry(alice);

}

Exercise SUT


bob.marry(alice);

}

Verify Result

As a developer,
I want to learn TDD,
so that I can write
clean code that works

Clean code that works
Clean code is simple and direct. Clean code
reads like well-written prose. Clean code never
obscures the designer’s intent but rather is full
of crisp abstractions and straightforward lines
of control

Grady Booch

Clean code always looks like it was written by
someone who cares. There is nothing obvious
that you can do to make it better. All of those
things were thought about by the code’s
author, and if you try to imagine improvements,
you’re led back to where you are, sitting in
appreciation of the code someone left for you,
code left by someone who cares deeply about
the craft.

Michael Feathers


You know you are working on clean code when
each routine you read turns out to be pretty
much what you expected.You can call it beautiful
code when the code also makes it look like the
language was made for the problem

Ward Cunningham

Simple design

The code is simple enough when it:
0. Pass all the tests
1. Expresses every idea that we need to express
2. Contains no duplication
3. Has the minimum number of classes and functions
(In this order)

Adapted from Extreme Programming Installed by Ron Jeffries et al.


• First we'll solve the “that works” part
• Then we'll solve the “clean code” part

Code that works could Smell

Refactoring by Martin Fowler

... then Refactor

Is the process of changing a software
system in such a way that it does not
alter the external behaviour of the
code yet improves its internal structure

Martin Fowler

through small steps

Refactoring by Martin Fowler

... ﬁght the Smells
Smell Common Refactorings
Duplicated Code Extract Method, Extract Class, Pull-Up Method, Template Method

Feature Envy Move Method, Move Field, Extract Method

Extract Class, Extract Subclass, Extract Interface, Replace Data
Large Class
Value with Object

Extract Method, Replace Temporary Variable with Query, Replace
Long Method
Method with Method Object, Decompose Conditional

... ﬁght the Smells
Smell Common Refactorings
Shotgun Surgery Move Method, Move Field, Inline Class

Replace Parameter with Method, Introduct Parameter Object,
Long Parameter List
Preserve Whole Object

Data Class Move Method, Encapsulate Field, Encapsulate Collection

Comments Extract Method, Introduce Assertion

an example of
Refactoring and
Unit tests

public class VobasBackupService implements Runnable {

public void run() {
Map fileDetails = new Hashtable();
try {
BufferedReader reader = new BufferedReader(new FileReader(MainFrame.WATCHED_DATA));
String directoryName = reader.readLine();
File fileData = new File(directoryName, ".vobas");
while (directoryName != null) {
if (!fileData.exists()) {
fileData.createNewFile();
} else {
ObjectInputStream fileDetailsReader = new ObjectInputStream(new FileInputStream(fileData));
FileDetail fileDetail = (FileDetail) fileDetailsReader.readObject();
while (fileDetail != null) {
fileDetails.put(fileDetail.getName(), fileDetail);
try {
fileDetail = (FileDetail) fileDetailsReader.readObject();
} catch (EOFException e) {
break;
}
}
}
File[] files = new File(directoryName).listFiles();
for (File file : files) {
FileDetail fileDetail = (FileDetail) fileDetails.get(file.getName());
if (fileDetail == null) {
ScpTo.send(directoryName + File.separatorChar + file.getName());
fileDetails.put(file.getName(), new FileDetail(new Date(), file.getName()));
} else if (file.lastModified() > fileDetail.getModificationDate().getTime()) {
fileDetails.remove(file.getName());
}
}
ObjectOutput objectOutput = new ObjectOutputStream(new FileOutputStream(new File(directoryName, ".vobas")));
for (FileDetail fileDetail : fileDetails.values()) {
objectOutput.writeObject(fileDetail);
}
objectOutput.close();
directoryName = reader.readLine();
}
reader.close();
} catch (FileNotFoundException e) {
// TODO Auto-generated catch block
e.printStackTrace();
} catch (IOException e) {

import java.util.*;

public class FileDetail {

make it
private Date lastModified;
private String fileName;

public FileDetail(Date lastModified, String fileName) {
this.lastModified = lastModified;

compile
this.fileName = fileName;
}

public Date getModificationDate() {
return this.lastModified;
}

public String getName() {
return this.fileName;
}

}

public class MainFrame {

}
public static final String WATCHED_DATA = "/dev/null";

stub
public class ScpTo {

public static void send(String filePathToSend) {

}
// TODO: no need to implement :-(
dependencies
}

is supposed
to work, but
how it works?

prepare your engine :-)
import org.junit.* ; import org.junit.* ;
import static org.junit.Assert.* ; import static org.junit.Assert.* ;

public class StubTest { public class StubTest {

@Test @Test
public void shouldAlwaysWork() { public void shouldNeverWork() {
assertTrue(true); assertTrue(false);
} }

} }

Untestable
public void run() {
try {
BufferedReader reader = new BufferedReader(new FileReader(MainFrame.WATCHED_DATA));
...

...
}
}

Extract Method
public void run() {
try {
dontKnowWhatItDoes(MainFrame.WATCHED_DATA);

...
}

public void dontKnowWhatItDoes(String directoryPathToBackup) throws ... {
BufferedReader reader = new BufferedReader(new FileReader(directoryPathToBackup));
...
}
reader.close();
}

doing refactoring
without tests
is unsafe

Characterization Test
public class VobasBackupServiceCharacterizationTest {

private List<File> directoriesToCleanup;

@Before public void setUp() throws Exception {
directoriesToCleanup = new ArrayList<File>();
}

@After public void tearDown() throws Exception {
for (File directoryToCleanup : directoriesToCleanup) {
deleteDirectory(directoryToCleanup);
}
ScpTo.sended = new ArrayList<String>();
}

@Test public void backupOneDirectoryWithOneFreshFile() throws Exception {
VobasBackupService service = new VobasBackupService();

File oneDirectoryWithOneFile = createDirectoryToBackupWithFiles(1);
File listOfDirectoriesToBackup = listOfDirectoriesToBackupIntoFile(oneDirectoryWithOneFile);

service.backup(listOfDirectoriesToBackup.getAbsolutePath());
assertEquals(1, ScpTo.sended.size());

directoriesToCleanup.add(oneDirectoryWithOneFile);
}

Characterization Test
@Test public void backupOneDirectoryWithTwoFreshFiles() throws Exception {

File oneDirectoryWithTwoFiles = createDirectoryToBackupWithFiles(2);
File listOfDirectoriesToBackup = listOfDirectoriesToBackupIntoFile(oneDirectoryWithTwoFiles);


directoriesToCleanup.add(oneDirectoryWithTwoFiles);
}

@Test public void backupTwoDirectoriesWithOneFreshFile() throws Exception {

File oneDirectoryWithOneFile = createDirectoryToBackupWithFiles(1);
File anotherDirectoryWithOneFile = createDirectoryToBackupWithFiles(1);

File listOfDirectoriesToBackup = listOfDirectoriesToBackupIntoFile(
oneDirectoryWithOneFile, anotherDirectoryWithOneFile);


directoriesToCleanup.add(oneDirectoryWithOneFile);
directoriesToCleanup.add(anotherDirectoryWithOneFile);
}

Magic Number



}

replace Magic Number with Expression


assertEquals(oneDirectoryWithTwoFiles.list().length, ScpTo.sended.size());

}

Syntax Noise
private void deleteDirectory(File directoryToDelete) throws Exception {
if (directoryToDelete.isDirectory()) {
String[] children = directoryToDelete.list();
for (int i=0; i<children.length; i++) {
deleteDirectory(new File(directoryToDelete, children[i]));
}
}

if (!directoryToDelete.delete()) {
throw new Exception("unable to delete " + directoryToDelete.getAbsolutePath());
}
}

replace For with Loop
for (File child : directoryToDelete.listFiles()) {
deleteDirectory(child);
}
}

assert directoryToDelete.delete() : "unable to delete " + directoryToDelete.getAbsolutePath());
}

Syntax Noise
String[] children = directoryToDelete.list();
for (int i=0; i<children.length; i++) {
deleteDirectory(new File(directoryToDelete, children[i]));
}
}

if (!directoryToDelete.delete()) {
throw new Exception("unable to delete " + directoryToDelete.getAbsolutePath());
}
}

replace Test with Assertion
for (File child : directoryToDelete.listFiles()) {
deleteDirectory(child);
}
}

assert directoryToDelete.delete() : "unable to delete " + directoryToDelete.getAbsolutePath());
}

now we can play safe
// read all directories to backup from a file
BufferedReader reader = new BufferedReader(new FileReader(directoryPathToBackup));

// for each of those directory

// get details on files in these directory
} else {
ObjectInputStream fileDetailsReader = new ObjectInputStream(
new FileInputStream(fileData));
try {
break;
}
}
}

// select only files to backup in directory
File[] files = new File(directoryName).listFiles(new FilenameFilter() {
public boolean accept(File directory, String fileName) {
return ! fileName.equals(".vobas");
}
});

// for each of those files

// if no previous details are given
// send to backup
// save details

// if details are given but file has been modified
// send to backup
// save details
}
}

// save all details on files to .vobas file
ObjectOutput objectOutput = new ObjectOutputStream(
new FileOutputStream(new File(directoryName, ".vobas")));
for (Object value : fileDetails.values()) {
FileDetail fileDetail = (FileDetail) value;
objectOutput.writeObject(fileDetail);
}
objectOutput.close();

// next directory to backup please...
}
reader.close();

narrow your target
and your tests
Comment
// read details on files in directory
} else {
try {
break;
}
}
}

Extract Method

} else {
try {
break;
}
}
}

Extract Method

public Map readDetailsOnFilesFrom(String directoryPath) throws ... {
File fileData = new File(directoryPath, ".vobas");
} else {
try {
break;
}
}
}
return fileDetails;
}

Extract Method


Map fileDetails = readDetailsOnFilesFrom(directoryName);

}
});

...

Duplicated Code, Complex Conditional
// send to backup

// save details

// send to backup

// save details
}

Consolidate duplicated conditional fragments
// send to backup

// save details

// send to backup

// save details
}


// send to backup

// save details


// send to backup

// save details

}

// if no previous details are given or
if ((fileDetail == null) ||
(file.lastModified() > fileDetail.getModificationDate().getTime())) {

// send to backup

// save details

}

Extract Method

if (noPrevioiusDetailsAreGiven(fileDetail) ||

fileHasBeenModifiedSinceLastBackup(fileDetail, file)) {

// send to backup

// save details

}
}

private boolean noPrevioiusDetailsAreGiven(FileDetail fileDetail) {
return fileDetail == null;
}

private boolean fileHasBeenModifiedSinceLastBackup(FileDetail fileDetail, File file) {
return file.lastModified() > fileDetail.getModificationDate().getTime();
}

Extract Method


if (needBackup(fileDetail, file)) {

// send to backup

// save details
}
}

private boolean needBackup(FileDetail fileDetail, File file) {
return noPrevioiusDetailsAreGiven(fileDetail) ||
fileHasBeenModifiedSinceLastBackup(fileDetail, file);
}

Rename Method

public void dontKnowWhatItDoes(String directoryPathToBackup) throws ... {
...
}

public void backupDirectories(String listOfDirectoriesToBackup) throws ... {



// send to backup

// save details
}
}

Extract Method

...

backupFile(file);
updateFileDetails(fileDetails, file);
}
}
...
}

private void backupFile(File fileToBackup) {
ScpTo.send(file.getAbsolutePath());
}

private void updateFileDetails(Map fileDetails, File file) {
}

Extract Method
BufferedReader reader = new BufferedReader(new FileReader(listOfDirectoriesToBackup));
String directoryPath = reader.readLine();
while (directoryPath != null) {
backupDirectory(directoryPath);
directoryPath = reader.readLine();
}
reader.close();
}

public void backupDirectory(String directoryPath) throws ... {
Map fileDetails = readDetailsOnFilesFrom(directoryPath);
File[] files = filesToBackupInto(directoryPath);


backupFile(file);
}
}
writeDetailsOnFilesTo(directoryPath, fileDetails);
}

Replace temporary variable with Query

File[] files = filesToBackupInto(directoryPath);


backupFile(file);
}
}
}

Replace temporary variable with Query


for (File file : filesToBackupInto(directoryPath)) {

backupFile(file);
}
}
}

Introduce Generics



backupFile(file);
}
}
}

Introduce Generics

Map<String, FileDetail> fileDetails = readDetailsOnFilesFrom(directoryPath);

if (needBackup(fileDetails, file)) {
backupFile(file);
}
}

}

Inconsistent Name
BufferedReader reader = new BufferedReader(new FileReader(listOfDirectoriesToBackup));
String directoryPath = reader.readLine();
while (directoryPath != null) {
backupDirectory(directoryPath);
directoryPath = reader.readLine();
}
reader.close();
}

Extract Method, replace For with Loop
public void run() {
try {
backupDirectories(listOfDirectoriesToBackup(MainFrame.WATCHED_DATA));
...
}
}

public List<File> listOfDirectoriesToBackup(String listOfDirectories) throws ... {
List<File> directoriesToBackup = new ArrayList<File>();
BufferedReader reader = new BufferedReader(new FileReader(listOfDirectories));
directoriesToBackup.add(new File(directoryName));
}
return directoriesToBackup;
}

public void backupDirectories(List<File> listOfDirectoriesToBackup) throws ... {
for (File directoryToBackup : listOfDirectoriesToBackup) {
backupDirectory(directoryToBackup.getAbsolutePath());
}
}

public void run() {
try {

} catch (Exception e) {
throw new RuntimeException(e);
}
}

backupDirectory(directoryToBackup.getAbsolutePath());
}
}

Map<String, FileDetail> fileDetails = readDetailsOnFilesFrom(directoryPath);

if (needBackup(fileDetails, file)) {
backupFile(file);
}
}
}

Feature Envy, Large Class
public void writeDetailsOnFilesTo(String directoryPath, Map<String, FileDetail> fileDetails) throws ... {
...
}

public Map<String, FileDetail> readDetailsOnFilesFrom(String directoryPath) throws ... {
...
}

private boolean needBackup(Map<String, FileDetail> fileDetails, File file) {
FileDetail fileDetail = fileDetails.get(file.getName());
}

}

}

private void updateFileDetails(Map<String, FileDetail> fileDetails, File file) {
}

Primitive Obsession, Data Clumps
public void writeDetailsOnFilesTo(String directoryPath, Map<String, FileDetail> fileDetails) throws ... {
...
}

public Map<String, FileDetail> readDetailsOnFilesFrom(String directoryPath) throws ... {
...
}

private boolean needBackup(Map<String, FileDetail> fileDetails, File file) {
FileDetail fileDetail = fileDetails.get(file.getName());
}

}

}

private void updateFileDetails(Map<String, FileDetail> fileDetails, File file) {
}

Extract Class
class BackupReport {

public Map<String, FileDetail> fileDetails;

private BackupReport(File directoryToBackup) throws ... {
fileDetails = new Hashtable<String, FileDetail>();
File fileData = new File(directoryToBackup, ".vobas");
} else {
ObjectInputStream fileDetailsReader = new ObjectInputStream(new FileInputStream(fileData));
try {
break;
}
}
}
}

public static BackupReport readFrom(File directoryToBackup) throws ... {
return new BackupReport(directoryToBackup);
}

public static BackupReport readFrom(String pathToDirectoryToBackup) throws ... {
return new BackupReport(new File(pathToDirectoryToBackup));
}

}

Extract Class

BackupReport lastBackupReport = BackupReport.readFrom(directoryPath);

if (needBackup(lastBackupReport.fileDetails, file)) {
backupFile(file);
updateFileDetails(lastBackupReport.fileDetails, file);
}
}

writeDetailsOnFilesTo(directoryPath, lastBackupReport.fileDetails);
}

Feature Envy


if (needBackup(lastBackupReport.fileDetails, file)) {
backupFile(file);
updateFileDetails(lastBackupReport.fileDetails, file);
}
}

writeDetailsOnFilesTo(directoryPath, lastBackupReport.fileDetails);
}

Move Method, Rename Method


if (lastBackupReport.needBackup(file)) {
backupFile(file);
lastBackupReport.update(file);
}
}

lastBackupReport.save();
}

Inconsistent Name


if (lastBackupReport.needBackup(file)) {
backupFile(file);
lastBackupReport.update(file);
}
}

lastBackupReport.save();
}

Rename Class, Replace Data Value with Object

public void backupDirectory(File directoryToBackup) throws ... {
DirectoryBackupStatus directoryBackupStatus = new DirectoryBackupStatus(directoryToBackup);

for (File file : filesToBackupInto(directoryToBackup)) {
if (directoryBackupStatus.needBackup(file)) {
backupFile(file);
directoryBackupStatus.update(file);
}
}

directoryBackupStatus.save();
}

public void run() {
try {
} catch (Exception e) {
throw new RuntimeException(e);
}
}

backupDirectory(directoryToBackup);
}
}

public void backupDirectory(File directoryToBackup) throws ... {
DirectoryBackupStatus directoryBackupStatus = new DirectoryBackupStatus(directoryToBackup);

for (File file : filesToBackupInto(directoryToBackup)) {
if (directoryBackupStatus.needBackup(file)) {
backupFile(file);
directoryBackupStatus.update(file);
}
}

directoryBackupStatus.save();
}

class BackupPlan {

public BackupPlan(File configuration) throws BackupFailedException {
this.directoriesToBackup = directoriesToBackup(backupPlan);
}

public void runWith(BackupService backupService) throws BackupFailedException {
for (File directoryToBackup : directoriesToBackup) {
new DirectoryBackupService(directoryToBackup).backupWith(backupService);
}
}

private List<File> directoriesToBackup(File configuration) throws BackupFailedException {
...
}

private List<File> directoriesToBackup;

}

class DirectoryBackupService {

public DirectoryBackupService(File directoryToBackup) throws BackupFailedException {
readBackupStatusFrom(directoryToBackup);
}

public void backupWith(BackupService backupService) throws BackupFailedException {
for (File file : listOfFilesToBackup()) {
if (needBackup(file)) {
backupService.backup(file);
update(file);
}
}
save();
}

...
}

Test-Driven
Development
Cycle

Write a test
public class AdderTest {
@Test
public void testTwoPlusThree() {
Adder a = new Adder();
assertEquals(5, a.add(2, 3));
}
}

Now it compiles
@Test
}
}

public class Adder {
public int add(int a, int b) { return 0; }
}

Red bar!
@Test
}
}

}

Expected 5, was 0

Just pretend
@Test
}
}

}

Remove the duplicated “5”
@Test
}
}

public int add(int a, int b) { return a+b; }
}

The procedure

1. Write a test

2. Make it compile
Expected 5, was 0

3. Make it pass

4. Refactor

Red

Refactor Green

Repeat every 2-10 min.

It’s not about testing

• TDD is a design technique
• The tests are not the main point
• The design emerges in small steps

Clean code, why?

• Design is the great accelerator:
• If you drop quality for speed, you will get neither
• If you aim for quality...
• ... and you know how to get it...
• ... you will also be fast!

Test ﬁrst, why?

• You think code from the point of view of the
caller
• This perspective makes for better design
• Test coverage is a useful byproduct

Refactor, why?
• What is clean code today could be bad code
tomorrow
• Refactoring is when I do design
• Design emerges, with thought, care and small
steps
• I don’t claim I can guess the right design
• Because I can: the tests support refactoring

No silver bullet

• Needs lots of practice
• Requires discipline
• Must think and be alert at all times!

Refactoring In Tdd The Missing Part

Refactoring In Tdd The Missing Part

More Related Content

What's hot

Similar to Refactoring In Tdd The Missing Part

More from Gabriele Lana

Recently uploaded

Refactoring In Tdd The Missing Part