Testing and Refactoring Legacy Code

In this video, we take a piece of crappy Java code with no tests. Our objective is to write tests for it and then refactor to make it better. The code has the most common problems that much larger legacy applications have, like Singletons, static calls and feature envy. It also has some design problems. Fixing that is quite hard, mainly when we need to write all the tests before we start the refactoring. Another rule: We cannot change production code if it is not covered by tests but quite often we need to change the production code in order to be able to test it. How to solve this problem? Well, I hope I can answer all these questions in this video.

If you want to know more about Software Craftsmanship, please check my book: https://leanpub.com/socra

If you want to play with the code, clone it from: https://github.com/sandromancuso/trip-service-kata

Hi, my name is sandro mancuso

in this screen cast

we're gonna take a piece of existing code without test

we're gonna write tests for it first

and when it is 100% covered

then we're gonna refactor it to make it better

so, the business requirements are

imagine a social networking website for travellers

so, you need to be logged in to see any type of content

and as soon as you logged in

if you want to see someone else's trips

you need to be friends with this person

it's kind of like your facebook stuff

so, if you're friends with someone you can see this person's trip

otherwise, you can't

so, but there are a few rules

we can not touch production code if it's not covered by tests

so, as we know, sometimes we should write test for existing code

we need to change the existing code

so the only exception to this rule is that you can use automatic refactoring

by our IDE, never type into the file

so, one approach that we gonna take is

imagine that this is how your code looks like

instead of trying to go to write test from the deepest branch

we're gonna choose the shortest branch of the code and write a test for that

and then we look at the second shortest branch

and then write a test for that

cause that gives us the ability to test by test

understand what our code does

and also we build up our test suite so we can get to the deepest branch

if we start to write a test according to the deepest branch

or trying to test the deepest branch

normally we need to create a massive setup to get there

so, we need to understand entire code base

so we prefer to do from the shortest branch to the deepest

let's see some code

so this is the trip service class

this is a server side class

and basically what it does is

it has a get trips by user

receive a user in there

so it checks the user session

gets the user that is logged in

then if the user is not logged in it throws an exception

if the user is logged in then

check if the logged in user is friends with the user passed as parameter

and if they're friends then I can retrieve

the list of trips for the user passed through parameter

so a bit confusing for now but what I'm gonna do

let's start writing test for it

may I just split the screen

that's normally how I prefer to code

I normally split my screen into two

I have my production code in one side

my test on the other side

then I don't need to keep switching from one to another

so that's my favorite way of coding

so, the first thing that we do

we just look for the shortest branch

we can ignore entire thing

I don't care what this does for now

for me, when I'm working with legacy code

the first thing that I do is

if I want to see what this code does

I look for the shortest branch

if the logged user is different from null

then it throws an exception

so if it is not logged in throws an exception

so let's write a test to see if that's true

so, should throw an exception when user is not logged in

let's put the exception here

expect it, user not logged in exception

so let's create the trip service

import that

now what we should do

trip service dot

get trips by user

pass as null here for now

I'm using Infinitest

so this red bar here basically show

everytime that I save my test or my production code

it runs my test for me

so I don't need to keep trying with my hands

this should be in theory

green

but something is happening

so if I run junit by hand

basically what's happening is

this should throw user not loggedn in exception

it's expecting user not logged in exception

but it's throwing a dependent a class during blah blah

so basically what's happening is

when this line here is executed

so when I say user session, get instance, get logged user

the user session

I did that on purpose of course

throws an exception

basically because in a unit test

we shouldn't be invoking other classes

because this may have the dependency of http session

it could go to the database to retrieve stuff

so we don't want that

so we need to break these dependencies

the problem that we have is

this is a singleton

and I cannot mock it right now

so I can't inject it, I can't type inside this trip service

but I need to find a way to get rid of it

I need to find a way to mock that

so the only things that I can do, as I said before

is I need to use automated refactoring

but I cannot not type

so here what I'm gonna do

I'm gonna create a seam

so seam is the division of where the two classes

the seam is where two classes meet

the trip service and the user session

so I'm gonna create this seam

so what I'm gonna do, I'm gonna create a method

saying get logged in user

and I will make that protected

so I just isolated the bit that goes to the other class

and that's my seam

so now what I can do as an intermediary step

I can create a private class, testable trip service

that extends the trip service

so what I do

I override the get logged in user

and I make it return null

and in my test, I replace the trip service by testable trip service

and so if I try to run that

now I have it green as you can see here

my infinitest somtimes goes crazy

sometimes I can trust, sometimes I cannot trust

in this case it was red but my tests are green now

so in theory I should be happy with that

just to confirm, so my test is green

and if I run my code coverage

so I make sure that I'm testing the shortest branch

so that's another good tip

cause every time that I work with legacy code

I always use code coverage

so lots people thing code coverage ...

ah... it's test coverage

I don't care about the percentage

the important thing for me here is, when I'm working with legacies

is my test covering the branch that I thought it would cover

because this avoid the false positives and negatives

so I know exactly what I'm doing now

so that's cool

so let's remove that the code ceverage

however this test is far from being good

because look at this

should throw an exception when user is not logged in

this test is all about the user not being logged in

but where does it say it in here

here we created the testable trip service

we invoke a method, pass null, throws an exception

but where's the user not logged in?

it's here, it's hidden

so we need to fix that

we need to introduce the concept

oh by the way, the reason that I have this weird notation in here

it's kind of weird for java people

but normally the Ruby they use this convention

because I can just collapse the method

and then I have the specification of my class

and normally I read like

trip service should do something

so that's how I normally prefer to create my test

so this is all about the logged in user that is not there anywhere

so I'm gonna create it

logged in user is null

so now I'm gonna create this guy

create field user logged in user

and I'm gonna say logged in user

so now it is little bit better because I introduced the concept that I was talking about

I don't like the nulls

cause null doesn't mean much in this case

so let's say logged in user is not logged in

or in fact in a web application

the opposite of logged in user is normally a guest

so here we take the opportunity start using the concepts that

you probably gonna speak the behavior to business people

right, so it started to use your domain language

in here, I have the user that should be friends with or not

in this case, I prefer to make it explicit that is

unused user

so now, see if I can

I'm just trying to see if I can make my Infinitest be happy

but it's not

so I won't be trusting my Infinitest unfortunately

so I much happier with my test now

and by the way my Infinitest is working again

so now my test reflex what I was talking about

so should throw an exception when user is not logged in

so the logged in user is a guest

and then throws an exception

so awesome

so what we can do right now is

rerun the code coverage

I just want to run the junit first

just to make it sure

cool

now I can run my code coverage

so now what I do is

according to my code coverage

I can easily see what is the second shortest branch

so basically, now is this for loop in here

and then I say if it's friend

so this is the deepest

cause to get to here this flag needs to be true

so basically what I need now is when the user is logged in

but he's not friends with the user passed as parameter

so we're gonna write the test for that

so let's keep the code coverage on

so let's say

should not return any trips when users are not friends, right?

so I'm gonna borrow these guys here for seconds

so now I need a logged in user

so I say, a registered user

let's create this guy

and here what I would need is a list trip trips

let's call it friend trips

so import all that

so basically what I need now

I need a proper user

I can't have a user in here

so I'll call it a

let's call it friend

so I need a user that is friend

so add friend, another user

friend, let's say add trip to brazil

so let's create these guys

ans so now I have a friend

that is friend with another user that has a trip to brazil

I have a logged in user that is a valid user

but they're not friends

so I need to assert that

friend trip dot size is zero

right, I could have said

assert that is empty doesn't matter

so my Infinitest that apparently is working

just confirm running the JUnit command

so it saying that it is green

that happens quite often in legacy code

cause the code is already there

you just making sure that you understand what code does

so that's quite common

I'm just run the code coverage

so running the code coverage now we progressed little bit

so as you can see my new test now moved forward

so went for the shortest branch

where the user was logged in

but they were not friends

so it was returning always false and then that it

so we're not done yet

because now we need to do some refactoring here

there is some duplication

so let's get rid of it

so let's put a before method

let's transform this guy

local variable into field

it's great

it's awsome

now we can move this guy up

right

so now I can go to the deepest branch

so now what I need is

when the logged in user is friends with the user passed as parameter

then I need the trips, return the trips

should return friend trips when users are friends

right? so that's the one we testing

so I'm gonna borrow this again

so you shouldn't do copy and pate

I don't do copy and paste

what you saw was an illusion

so I never, never ever do copy and paste

so in this case here what I want you to do now is

friend, add friend with logged in user

let's add another trip

so now of course that I expect my Infinitest are red saying with this

red, of course I expect this should be true

because now the users are friends

and if I save

it still saying that it's red

so let's run junit by hand

oops, I keep getting the shortcuts wrong

because I kept using windows

my mac and intellij keep getting (???)

so what it's saying is, because now what's happening

just restore this screen

what's happening is

the code is now getting here in the trip dao

that is a statical

and as soon as the code execute this guy

then there's an exception being thrown

now I give the same thing

so this is just to prove the point that

just to make a point

that we shouldn't ever be calling or execute another class in a unit test

because this, in the real world, would be going to the database and fetching data blah blah blah

the test for my trip service shouldn't care about that

so we need to do exactly the same thing

that we've done for user session

so you can break the hard wired dependencies

like singleton and staticals

or objects being created inside method is exactly the same

so basically what we do

create a method

let's say trips by user

that is also protected

and then we go back to our tests

we'll do the same thing

so basically what we'll do

we will just return

so we will receive the user

the user in our test that has two trips

so basically we just return that

the user dot trips

so as soon as do that my test is green

so if I run my code coverage

awesome

cause now I have all my existing code base

tested to almost 100%

of course that you might say that I created are not test because I overriding them

but that's alright because that's just my seam

that's a very thin layer to delegate

and that's intermediatary stage anyway

so now few clean ups that we can do

normally what I do is

because this guy is here and is here

and the only situation that I don't want it should be like that is this one

so I leave this one

and I just move this one out

because then it is common to all other tests

all other tests that I want the logged in user should be a valid one

so once I do that

if I save

then if I just make it sure

so all my tests are green

and that's great

so for the first part, it's great we're done

now our code is tested and we're free to start refactoring

so now that we're in a very good point to commit

so git status

git commit

unit tests for trip service

cool, awesome

look at this test, the problem is

legacy code is that quite often you need

this is of course very simple one

but imagine like a same method with hundreds of line

and here we're just talking about one object being created

to pass in as parameter

but quite often what we do is we need to create this massive object graph

to pass in as parameter

so we can reach all the branches and stuff

so we can start cleaning that up

cause I don't like all these duplication in here and here

so the way that I would love to see it

is using a builder

so basically what I loved to see is something like that

user friend

kind of like a friend or a user

friends with another user and logged in user

with trips to brazil and to london

so that's the sort of stuff that I would like to see

I think I read far better

so let's try to see if we can make that happen

so let's put a build method in here

let's use the builder pattern for that

let's say user builder

so that would be what I want to see

so I will gonna comment this out a little bit

so I'll say, public, static

class, user builder

let's start creating this method in there

the way that you can chain methods

you'll just keep returning the same object

and then you can chain all the methods

so let's create the use builder

so now we just create the next method

keep returning the user builder

so here we're passing two users

as I always said, there are just three numbers in programming

there is 0, there is 1 and there is many

there's nothing more than that

so let's make it many

so let's use variable arguments

cause that I can pass many users as I want

so let's put friends

let's say these friends equals friends

let's keep returning same objects

let's create this guy

should never leave arrays uninitialized

so new user

avoid some null pointer exception

so great, let's go for our second method

so user builder

let's do the same thing

because then we're free to use our method to pass as many trips as we want

let's return trips

let's create trips

normally what I do

I write the way I want to read it

and then I start creating all the code from there

and let's create the build method

so build method, I like it to be the last method

so the build method is the method that actually build the object

so new user

let's return the user

and then I say

add trips to user

add friends to user

it's always to cool to combine the method name and the parameter

it makes it much easier to read it

avoid duplication in names as well

so let's do the trips first

so for each

here we're doing the trips

so that's a trip that's a trip that's trips

so user dot add trip, trip

let's do the same thing for friends

create add friends

let's do

for each

so that's user

that's friend

friends

user dot add friend friend

awesome, cool

so now, Infinitest saying everything is green

so I can delete this

and I can replace this one as well

so let's replace this one

awsome, let's comment that out

so if I run everything, still green

again, awesome

so now my tests are slightly more readable

so what I will do move this builder out

so let's say

move type to new file

and basically what if you do

if you just move this class the same folder where the TripServiceTest is

for now it's good

so now that is out

static import this methods in here

static import this methods in here

so let's make code symmetric

so both of them are symmetric

and let's remove all the unutilized import

so there's no unused imports anywhere

so excellent

so now I'm much much happier with my test

because then I can read it properly

and that's a great point to commit as well

so let's see

we got status

git add

git commit

user builder created and used in trip service test

cool, awesome

so now we're finally ready to start refactoring this

as we're saying before

where was it, here

we normally start testing from the shortest branch to the deepest

and when we're doing refactoring it's the other way around

you start from the deepest branch

to the shortest delta branches

the reason beneath is

if you want to start refactoring in the middle of your code

you need to understand everything

and normally

legacy code, they have global variable everywhere

and dependencies

set a flag in one place, checks it on the other

but if you go straight to the deepest branch

the deepest branch doesn't depend on anything

it normally receives everything that it needs that is needs

and then it does something with it

if you're able to extract that

refactor that the deepest branch from deepest to shortest

you're code is start shrinking

so let's look at the code

so the deepest branch of the code

as we discussed before is this one

so to get to here

I need this flag to be set

the flag is set here

the problem in this example in specific

the deepest branch that is this line

doesn't do much

it just delegates to this one

so I can re-do anything with one line

so the second deepest branch is this one

right it is this bit in here

so let's go for this one

one thing that is very very important

when working with legacy code

is to understand why the method is so big

why is it so complicated?

and normally it is because the method, of course, does too much

that's why it has so many lines

so it's important that instead of us trying to make the algorithm better

or extract method in here

we should ask ourselves

does this responsibility

does this behavior belong here?

in this case, look at this

if you look at this block

so we go to the user object and asks for the friends

so we get a list of friends out of the user object

we iterate through it

just to ask if the logged user is inside that collection or not

so this is called feature envy

it's kind of the trip serivce envies the user

it wants to be the user class

so what we could do here, we need to solve the feature envy

if the user class has the collection of friends

we should ask the user class, like

user are you friends with this guy?

so, let's try to move this behavior there

so the first thing is

let's look at the user class

where's my user?

so let's close this too

so that's my user class

let's create a test for it

so my user class is here

new class, let's say user test

but I'm gonna put that in the test package of course

so that will create my user test in here

let's move my test here

cool, so what do I want?

I want this behavior here should be in this class

I want to go to the user and say,

"Are you friends with another user?"

right?

so, I'm gonna write this test

should inform when users are not friends

I'm gonna start with not friends

because it is the easiest test that I can write

this is the simplest one

here I can use my builder

user builder dot a user

so let's say that this guy is friends with Bob

right? let's do that

and then in my test, I'm gonna say

assert that user is friends with Paul is false

so basically I want to create a user that is friend with Bob

and of course if I ask this user if he's friend with Paul

it should be false

so let's create Bob and Paul

let's create Paul as well

so now that's the method that I want the user class have

let's create it

and of course the minimum behavior that I can have to satisfy the test

is false

so as soon as I save the false

my Infinitest tells me that it's passing

that's awesome, great

so let's do the other case, when the users are friends

it should inform when users are friends

so I'm gonna borrow these guys here

let's move that down

and what I'm gonna do is

you're also friend with Paul

and I expect it, of course

it should be true

so my Infinitest are saying that it's not true

now I'm gonna just implement

just friends dot contain

Paul doesn't work here

let's say another user

cool, so now I have my method in there

awsome so of course that is covered

there are ones that are not used in this one

that's alright

so this is a great point to commit

git status

git add

so git commit

is friends with method added to user

awesome

so now we can go back to our test

just remove all the recorded code coverage marks

now I have the user with the behavior that I want

so when we start to refactoring

we should try to stay in the green

for as long as possible

instead of going crazy in the refactoring

and say I'm gonna change everything

and then all the tests are broken

we should, every single step that we make

we should run our tests

using Infinitest, is a way works well

is fantastic because that

if you make one move save it and it runs your test

and then you know where you are

and as soon as you make one move

something is broken

so your test shows the red in there

you're one Ctrl+Z away to be back to green

you just Ctrl+Z and save it

you're back into green

so let's try to make the smallest step that we can

to do this refactoring and stay in the green, right?

so what if I say

what if I first of all move this boolean in here

because it just used inside of the if statement so I can bring

oh, there's another tip

when working with legacy code

normally you find variables declared all over the place

try to bring them together

try to put the blocks of code close each other

if the variable is not used at the top

it's just used at bottom

but it's declared at the top

bring that close

because it ??? know where the blocks are

and it makes you much easier to separate the code

so now I could say

is friend, I could say

user is friends with logged user

let's rename logged user to logged in user

so that's how I think

when you're dealing with legacy

you find names are quite weird or not quite right

as soon as you figured out where they are

just rename it quickly

so cool, as you can see

you can keep an eye on the green bar here

so every time that I saved it

it's gonna run my test

so now, I believe that is quite safe

to do this and save

so it ran my tests and that's green

so now I can delete that

so now that I can delete

I maybe able to

let's try to inline this

awesome

so that's cool

so there is a quick win as well here

for example, this guy here

it's a guard clause

we can transform this into guard clause

what is a guard clause?

as soon as parameter comes in

you invalidate parameter and throw an exception

that's the guard clause

so what we can do

let's transform that to guard clause

we can get rid of if

so if logged in user is null

then

let's throw an exception

of course that this guy need to be up

so as soon as I saved it

so I'm still in the green

and Eclipse kindly showing to me that this is dead code now

so I can delete the entire if statement

so if I save it

awesome

so I'm doing all these refactoring without going to the red

not even a single time

but even if I make a mistake and I go to the red

what normally happens in the real world

that's alright, I do a Ctrl+Z

I'm back into green

and I re-think what I'm doing

sometimes you need to go to the red

but I avoid it as much as I can

so now we have two clear blocks in here

the first block is about checking or validating the logged in user

the second block is about getting the trips from the friend

so we can do a few things slightly better in here

so for example I could say

getting rid of variables in legacy code

is another thing that you should try to do

because the less variables you have the easier it is

even if you have some redundancy in there

if you call method twice

you may sacrifice performance for a while

but getting rid of variable

cause variables are the most people ??? in long method

because they're set in many different places and reused

if you can get rid of them

and just invoke the same method over and over again

it'll be easier to read all the code

and then you do the optimization for ??? again, if you need to

so for example, what I'm trying to do here is

so saved it

I'm still in the red

so I believe that what I could even do is say

return

so I'm still in the green

so return

of course it don't need

so if I save that I'm still in the green

and I don't need the variable

don't need that

still in the green

and now that I have symmetrical

if I can use ternary operator

so I could do that for exmple

hopefully if I save that

I'm still in the green

awesome

another thing is

this variable here is used in two different places

so let's inline that

inline from here. yes, I can

it's inlined in both places

that's what I was talking about

I got rid of the variable

and of course I'm calling the method twice

but I made my code a lot simpler

of course if there's performance issue

then you extract variables again all sort of stuff

so that's awesome because now I'm much happier

and one thing that I don't even like this guy here

let's extract method in here, and say

no trips, let's make it private

so the cool thing about it is

that for example

if we look at the code

if I look at my test now

should throw an exception when user is not logged in

get logged in user

throw an exception

should not return any trips when users are not friends

if user is friends with logged in user

no trips

should return friend's trips when users are friends

so if user is friend with logged in user

trips by user

so you end up saying the same language used in the test or test names

being reflected in your code

how your code is written

that's one of the things that I want you to achieve

so that's another good place to stop and commit

so let's do that

so git status

git commit

trip service refactored

cool

so that's awesome

so we may think that this is good, right?

we are done

but NO.

we are far from being done

see, there's tiny piece of code

but for example, we have enormous problems

one of the problem is

what we've done here is

we took a piece of untested legacy code

and then we wrote test straight

and when we do that

that's great, that's awesome

cause then we feel confident that we can change it

(???)

but the drawback is that

what if the design is wrong?

cause if the design is wrong

what we're doing when we write test

we're perpetuating that design

so we need to be very careful

because if we're not very inclined to change the design

and now we have a battery of test covering that stuff

we're far less inclined now(???)

because as soon as we start to change design

you'll break all those tests

so you need to be careful with that

and why the design here is wrong?

well, by the name

I assume the trip service is a server side class

belongs to the Model

but then it has a dependency on the user session, right?

so your Model shouldn't know anything about the web framework

shouldn't know anything about http session or anything like that

so, that's the problem with the design that this class has

so we need to solve that

one way to solve that is

we should pass the logged in user as parameter to this method

so now I'm gonna make sort of dangerous refactoring

that it will be ok for this exercise

but if you do that in a normal production environment

you need to be very careful

so I want to pass

I'll try to do, as always

baby steps, bit by bit

so for example I want to pass the logged in user as parameter

so what I can do is

I go to refactor

change method signature

and I'm gonna add a new parameter

I'm gonna call user

logged in user, right?

and the default is null

so that's cool

because eclipse does great job in refactoring

IntelliJ does same

so this guy was passed in as parameter

it's not used anywhere else

but it changed in my test

it changed the method signature to pass null, everywhere, right?

so that's cool, that's great

the only problem is, for example

this method here being a production method

it's probably used

or it's probably invoked by other classes

and now all the other places where this method here is invoked

the production code is passing null as well

so you need to go back there and fix that

so you just need to bear that in mind

but for the sake of this exercise

so what we want to do

we want to do fix our test

cause we don't want this nulls hanging in around

and this is the logged user

so I'm gonna pass here, a guest

because that's what I would pass

so as always I'm looking at my Infinitest

so here I'm gonna pass my logged in user

here I'm gonna pass

yeah, I'm gonna pass my logged in user here

so that's cool

just to make sure that everything is ok

so cool

now I want to start using this parameter

so what I'll do

I'll just use the parameter

I'm just gonna rename that to this

rename that to this

so this is the parameter now

so if i save it

and so all my tests are green

so that means that this guy

transform that in private

so eclipse say that it's not been used

so I can delete safely

and of course, as soon as I delete my test

I just need to delete this guy

and everything is back to green

so now I'm using the parameter

I made my refactoring

(???) thing that I can do is

this logged in user

I probably don't need that anymore

so if I replace this one by these guys here

and I put this guy here

so one way to know if

so now for example, eclipse is saying that this variable is not being used

so now I can delete that

and delete that

delete that

and everything is green

so this is a perfect place to commit again

oh, by the way, there is a warning here

so should always get rid of the unused import

because rubbish thing around (???)

so craftsman ship is important

keep it clean

so cool, that's a great place to stop

so let's say

git commit

logged in user passed into trip service

cool, awesome

so maybe now we're done

sort of, sort of

I still don't like this thing in here

this trip dao thing

I don't like the statical

this statical makes me do this thing that

as I said this is sort of intermediately step

ideally I would inject trip dao in there

using an instance method

but because it's static method, I can't do much

I can't really inject that, right?

cause this is always

you mean normally inject an instance not class

so it doesn't work

so I want to go further

I want go get rid of this

I want to have a instance method in my trip dao

so let's open my trip dao

let's see what it does

so of course that in this case

it is just like a toy example

so you just throw an exception

as I said before in a normal circumstance

this would go to database

or to distributed cache

or whatever persistence mechanism you have

so what I want to do

I want to create an instance method for that

so I'm gonna write a test for it

let's say

trip dao test

let's put into test

so this is an interesting one

it's an interesting trick

so I want an instance method

that does everything that the static method does

same behavior, just

you could say, well, just remove static method

just remove the static keyword from there

yes, I could

but if I do that

I'll need to fix my entire code base

because, let's assume that there are many classes in my application

that are using this static method

as soon as I transform this one into instance

I'll break entire code base

and that'll be much bigger refactoring to do

and we want to do everything in small steps

so what I'll do

I create a test, let's say

I'll just mimic this behavior

so don't take this test too seriously

just get the idea of what I'm doing

but I'll just represent what this method currently does

so should throw exception when retrieving user trips

so I want this to throw an exception

as I said

ideally this would go to the database or something

I'm just preserving the same behavior

so dependent class call during unit test blah blah

so that's the exception thrown

but I want an instance method

so just invoking the method should be enough

so what I can do

new trip dao

let's call it trips by user

so I want an instance method

that throws an exception

so I'm gonna create this method

I'm gonna make it return the same thing as the other one

so my test is failing, of course

so now what I want this to do

it should behave exactly like this static method

so what I do is trip dao dot

I just invoke from the instance method

invoke the static one

why do I do that?

the cool thing is that

as I start moving my code to use this one

so now what I'm gonna do

I'm gonna replace the trip service used the instance method

and imagine that I start doing that

for all the other class that used static

until there's no class referencing the static method anymore

and what I can do

I just can copy this behavior

paste it into here and delete the static method

and then I'm done

so that's the trick

so going back to

so now my trip dao has the instance method

so what I need to do now

is to have a reason to inject this trip dao in there

so now I'm gonna

there are quite of few ways of doing that like for example

I could use normal dependency injection with no framework

just pass the trip dao into the constructor of the trip service

for example, that's one way of doing it

and that's the simplest way

as I know that majority of people that use java

they use spring and they use mockito and stuff like that

I'll show you how it would work in mockito

but for now, if you don't use mockito

then that's it

create a constructor pass the trip dao to the constructor

I'm just gonna do with mockito and spring

just to show what you can do basically

so I want now to have a broken test

that justifies me to inject a trip dao in there

so, I'm gonna say run with

I'm gonna use JUnit with mockito

mockito runner

junit runner

so if you don't use mockito and the junit in spring (???)

maybe a bit complicated

so I recommend that you read the documentation of all these frameworks

so far, I've been using the testable trip service

that class we created before

now I want to start using the real one

the real one and injecting

cause I don't want these thing here anymore

I don't want this madness here anymore

I want to just to be able mock to my dependency

so I'll create another

do that in very small steps

trip service

I'll call for now real trip service new trip service

that's the real one

I'm gonna create a mock

I'm gonna create a mock to my dao

I'm gonna use mockito, to say

inject mocks

and I'm gonna spy on it

so basically it's out of scope for me to explain how mockito works

but basically in very short explanation

I'm creating the real class

I'm asking mockito to spy on the real class

and to inject all the mocks that this class may have

I create the mock in here

I just need to inform that this class is expecting this mock

so but before I go there

I'm gonna, now I'm using the real trip service in here

I'm gonna where it breaks

so for example

I can go bit by bit, I can say

real trip service and save

so my test still green

so because what I'm trying to do

I'm trying to get rid of this guy now

bit by bit

to replace with a real one

so real trip service

so if I do that

it works, cool

the reason it works

because none of those test that use, they go the dao

none of them get to this line

so this one, real trip service

so now it breaks, right?

that's what I was looking for

now it breaks because

I'm using in the test the real trip service

and that calls the static method in the trip dao

and that throws an exception

so cool, now I have a broken test

so I have a reason to change my production code

so what I'll do

I'll go to my production code and say

if I'm using spring

auto wired

so just import the auto wired

so imagine that if you gonna define the trip dao in a spring bean

in a xml

so private trip dao trip dao

awesome

maximize that

so cool

so what mockito will do

mockito will take this guy here

mymock and inject into this guy here

so now I need to start using that

so my test is broken

and I need to start using that

so in here, instead of using the

so if I run, I just run a test

just to show you the difference

run as

I keep pressing wrong button

so it's due to using the real dao, right?

so what we're gonna do is replace with the instance

and say use the instance method

instead of this classic one

so now if I run my test again

the error changed

so basically it's now using my mock

so if I just minimize it again

so now it's using my mock

and it says that the test expects two trips but it got zero

so that's what my test does

my test's expecting two

so what I need to do is

the reason that this is happening

because now my production code is using the mock

and the mock is not configured

so I just need to say that

given my trip dao

dot trips by user

so given that this method is invoked in my mock

it will return friend dot trips

so what I'm doing here

I'm configuring my mock to return the friend trips

exactly as the way we've done here

I'm doing in here just configuring my mock

so now as you can see my test is green

so this guy can go back to private now

and I don't need this method anymore

and if I run all my tests

my tests are all green

so that means I don't need this class anymore

so that means that I don't need this

and if I don't need this, I don't need this

and in this case, this guy is not been used anymore

I can delete

so now I can remove

I can rename my real trip service

because real means nothing

so when we're doing test

I've seen people calling variable like testee or mock (???)

call it what it is

it's a trip service. so it should be called trip service, right?

so now it's great because

I don't have that madness of test double class anymore

my test reads quite well

and so just make sure that everything is green

I can even go there and say

so validate logged in user

so now as I said

I can close all my test methods

and we can read this block here and say

should throw an exception when user is not logged in

that's part of the validation of the logged in user

should not return any trips when users are not friends

so if the users are not friends returns no trips

so should return friend trips

when users are friend

so if the user is friends with the logged in user

just return the trips

even this getter method here is not great

so get trips by user so that could say

get friend trips

that sounds much better

this guy could be

friend as well

or something like that

so you can play with these things now

that you have all the code coverage

so basically that's another great point to stop

so git status

git add .

so git commit

trip dao injected into trip service

so this it it

that's normally

that's what all the things that you can do

with legacy code in a very safe way

I can show you

I have original trip service somewhere in here

so you can compare

so we went from this to this

and of course that

it took me just an hour to do that

but it's because I'm recording this screen cast

but as soon as you're comfortable with all these techniques

this probably would have taken me in a normal day like 20 minutes

so it's just like

as soon as you're aware of what is available to you

then you just practice it

and then it makes things much easier

and just as a reminder

every time that you're working with legacy code

your first test should never be the one that goes to the deepest branch

always start from shortest branch

from the shortest to the deepeset

write your test in this order from the shortest to the deepest

it's easier because the test will help you to understand what the code does

and it will help you to build the test data

bit by bit as you dive deep into the code

when you're refactoring

do the other way around

find the deepest branch

and start extracting method from there

starting put in that behavior into different classes

and then all of sudden your code will start

collapsing and getting smaller and smaller

because you're removing the internal parts

so try to write readable and maintainable code

try to have the domain language in there

try to capture that

like using things like guess(???) and stuff like that

so as part of the domain

try to bring the domain language into your test and into your code

the language spoken in the test and the code and used it with your BA should be the same

try to have simplicity

know your shortcuts, frameworks like

try to be fast

configure your environment so you can with a few shortcuts

create test methods and classes and all sort of stuff

and do the refactorings

and as I said working in very small increments

commit often

I mean if you're using git and stuff like that

because if you make mistake

Ctrl+Z or git reset hard

you go back to previous commit

and you're back into the green

be brave

now you know a lot of things like

all these small tricks that we learned today

we'll make probably 99% of all your classes testable

so just be brave

use it, and

always leave the campground cleaner than you found it

so that's the boy scout rule

no broken windows

if it's smelly

if it's not very well written

just refactor it

just go for it

yep, so I hope you enjoyed it

that's it

so you can find this trip service kata in my github

so just go to github sandromancuso trip service kata

that's it, hope you enjoyed it

thank you