October 17, 2017

Learn TDD with Codemanship

Manual Refactoring : Convert Static Method To Instance Method

In the previous post, I demonstrated how to introduce dependency injection to make a hard-coded dependency swappable.

This relies on the method(s) our client wants to invoke being instance methods. But what if they're not? Before we can introduce dependency injection, we may need to convert a static method (or a function) to an instance method.

Consider this Ruby example. What's stopping us from stubbing video ratings is that we're getting them via a static fetchRatings() method.



Converting it to an instance method - from where we can refactor to dependency inject - is straightforward, and requires two steps.

1. Find and replace ImdbRatings.fetchRating( with ImdbRatings.new().fetchRating( whereever the static method is called.

2. Change the declaration of fetchRating() to make it an instance method. (In Ruby, static method names are preceded by self. - which strikes me as rather counterintuitive, but there you go.)



NOW RUN THE TESTS!

If fetchRating() was just a function (for those of us working in languages that support them), we'd have to do a little more.

1. Find and replace fetchRating( with ImdbRatings.new().fetchRating( wherever that function is called.

2. Surround the declaration of fetchRating() with a declaring class ImdbRatings, making it an instance method.

(AND RUN THE TESTS!)

Now, for completeness, it woul make sense to demonstrate how to convert an instance method back into a static method or function. But, you know what? I'm not going to.

When I think about refactoring, I'm thinking about solving code maintainability issues, and I can't think of a single maintainability issue that's solved by introducing non-swappable dependencies.

When folk protest "Oh, but if the method's stateless, shouldn't we make it static by default?" I'm afraid I disagree. That's kind of missing the whole point. Swappability is the key to managing dependencies, so I preserve that by default.

And anyway, I'm sure you can figure out how to do it, if you absolutely insist ;)



October 16, 2017

Learn TDD with Codemanship

Manual Refactoring : Dependency Injection



One of the most foundational object oriented design patterns is dependency injection. Yes, dependency injection is a design pattern. (Not a framework or an architectural philosophy.)

DI is how we can make dependencies easily swappable, so that a client doesn't know what specific type of object it's collaborating with.

When a dependency isn't swappable, we lose flexibility. Consider this Ruby example where we have some code that prices video rentals based on their IMDB rating, charging a premium for highly-rated titles and knocking a quid off for poorly-rated ones.



What if we wanted to write a fast-running unit test for VideoPricer? The code as it is doesn't enable this, because we can't swap the imdbRatings dependency - which always connects to the IMDB API - with a stub that pretends to.

What if we wanted to get video ratings from another source, like Rotten Tomatoes? Again, we'd have to rewrite VideoPricer every time we wanted to change the source. Allowing a choice of ratings source at runtime would be impossible.

This dependency needs to be injected so the calling code can decide what kind of ratings source to use.

This refactoring's pretty straightforward. First of all, let's introduce a field for imdbRatings and initialise it in a constructor.



NOW RUN THE TESTS!

Next, introduce a parameter for the expression ImdbRatings.new().



So the calling code decides which kind of ratings source to instantiate.



AND RUN THE TESTS!

Now, technically, this is all we need to do in a language with duck typing like Ruby to make it swappable. In a language like, say, C# or C++ we'd have to go a bit further and introduce an abstraction for the ratings source that VideoPricer would bind to.

Some, myself included, favour introducing such abstractions even in duck-typed languages to make it absolutely clear what methods a ratings source requires, and help the readability of the code.

Let's extract a superclass from ImdbRatings and make the superclass and the fetchRating() method abstract. (Okay, so in C# or Java, this would be an interface. Same thing; an abstract class with only abstract methods.)



DON'T FORGET TO RUN THE TESTS!


One variation on this is when the dependency is on a method that isn't an instance method (e.g., a static method). In the next post, we'll talk about converting between instance and static methods (and functions).



October 12, 2017

Learn TDD with Codemanship

Manual Refactoring : Extract Class



A core principle of software design is that modules (I'll leave you to interpret that word for your own tech stack) should have a single distinct responsibility.

There are two good reasons for this: firstly we need to separate code that's likely to change at different times for different reasons, so we can make one change without touching the other code. And it provides us with much greater flexibility about how we can compose systems to do new things reusing existing modules.

Consider this simple example.



Arguably, this Python class is doing two jobs. I can easily imagine needing to change how movie ratings work independently of how movie summaries work.

To refactor this code into classes that each have a distinct single responsibility, I can apply the Extract Class refactoring.

First, we need a new class Ratings to move the ratings fields and methods to.



NOW RUN THE TESTS!

Next, paste in the features of Movie we want to move to Ratings.



AND RUN THE TESTS!

Now, we need to substitute inside Movie, delegating ratings methods to a field instance of the new class Ratings.



RUN THE TESTS!

Okay, so - technically - that's Extract Method completed. We now have two classes, each with a distinct responsibility. But I think we can clean this up a bit more.

First of all, another core principle of software design is that dependencies should be swappable. Let's introduce a parameter for ratings in Movie's constructor.



We can now vary the implementation of ratings - e.g., to mock or stub it for testing - without changing any code in Movie.

If Movie was part of a public API, we'd leave those delegate methods rate() and average_rating() on it's interface. But let's imagine that it's not. Could we cut out this middle man and have clients interact directly with Ratings?

Let's refactor the test code to speak to Ratings directly.



AND RUN THE TESTS!

Now, arguably, the first two tests belong in their own test fixture. Let's extract a new test class.



RUN THE TESTS!

Then we can remove the now unused delegate methods from Movie.



DON'T FORGET TO RUN THE TESTS!

And, to finish off, put each class (and test fixture) in its own .py file.

AND RUN THE TESTS!

This was a fairly straightforward refactoring, because the methods we wanted to move to the new class accessed a different field to the remaining methods. Sometimes, though, we need to split methods that access the same fields.



If I extract a new class for generating HTML, it will need to access the data of the Movie object it's rendering. One choice is to pass the Movie in as a parameter of to_html().



This has necessarily introduced Feature Envy in HtmlFormatter for Movie, but this may be a justifiable trade-off so that we can render movies in other kinds of formats (e.g., JSON, XML) without changing Movie. Here we trade higher coupling for greater flexibility.

In this refactored design, Movie doesn't need to know anything about HtmlFormatter.

Whether or not that's the right solution will depend on the specific context of your code, of course.



October 10, 2017

Learn TDD with Codemanship

Manual Refactoring - Summary

Due to the increasing popularity of dynamically-typed languages like Python and Ruby, as well as a growing trend for programming in stripped-down editors like Atom, Vim and VS Code that lack support for automated refactoring, I'm putting together a series of How-To blog posts for script kiddies that need to refactor their code the old-fashioned way - i.e., by hand.

The most important message is that manual refactoring requires extra discipline. (I often find when I'm refactoring by hand that things can get a bit sloppy, and I'm sure if I watched it back, the code would be broken for much longer periods of time.)

So far, I've done 12 posts covering some key refactoring basics:

Rename

Introduce Local Variable

Introduce Field

Inline Variable & Simple Method

Inline Complex Method

Introduce Parameter

Extract Method

Move Instance Method

Extract Class

Extract Superclass

Dependency Injection

Convert Static Method to Instance Method

In coming days, I'll be adding to this list, as well as putting together my definitive guide for manual refactoring, which may become an e-book, or a commemorative plate, or something like that.



Learn TDD with Codemanship

Manual Refactoring : Move Instance Method


I good deal of the refactoring I do involves shifting responsibilities between the classes in my code to try and minimise coupling between them.

When I see an example of Feature Envy - when a method of one class relies heavily on features of another class - I consider moving that method to where it arguably belongs.

Moving instance methods can be simple, or it can get complicated. Let's start with a simple example.



Here, the area() method of CarpetQuote only uses features of Room. It's only used internally within CarpetQuote, so if I moved it to Room no client code would break. Also, area() uses no features of CarpetQuote, so there'd be no need to create a dependency pointing back the other way.

To move an instance method, there needs to be an instance to move it to. This becomes the new target of any invocations. (i.e., self.area() becomes self.room.area() ).

First, I cut the area() method code and paste it into the Room class.



Then I change references to Room's getters to point at the equivalent internal fields of Room.



Then I change any invocations of area() in CarpetQuote to the new target instance, which i this case is the room field.



NOW RUN THE TESTS!

The accessors get_width() and get_length() are no longer being used, so I can inline these, moving our design into a more "Tell, Don't Ask" style.

As well as being able to move instance methods to fields, we can move them to method parameters. Imagine a slightly different version of our original example, where the Room is passed in to the area() method.



This time, when we move area() to the Room class, the target instance is the parameter value self.room. The invocation is turned around and we remove that parameter, so self.area(self.room) becomes self.room.area().



AND RUN THE TESTS!

Now let's make things a little more complicated. What if area() is called outside of CarpetQuote?

There are three potential solutions here:

1. Leave area() on CarpetQuote as a delegate to room.area()



(RUN THE TESTS!)

The advantage of this approach is when we need to preserve a public API. It leaves the client code untouched, though arguably is less clean as it creates obtuse indirection in our code. If this wasn't a public API, there might be a better way.

2. Expose the room field so the calling code can access it.



(AND RUN THE TESTS!)

Yuck. This is my least favourite solution, although it does work.

3. Switch invocation targets in the client code.



(RUN THE TESTS!)

This, in my opinion, is the cleanest solution. It requires a bit more work, but gets the client talking directly to the object it wants to talk to, and removes the unnecessary "middle man", making the code simpler.

Finally, we need to consider what to do when a method we want to move has internal dependencies.

Imagine when we calculate the area of carpet required, we need to subtract an uncarpeted border that the customer has asked for (i.e., not wall-to-wall carpet).



If we wanted to move area() to Room now, we could go about resolving the references to border in two ways.

1. Pass a reference to this instance of CarpetQuote as a parameter of area()



So Room can now resolve that reference.



(RUN THE TESTS!)

But this bi-directional dependency is less than ideal, and could create problems for us later.

2. Resolve the reference in CarpetQuote and pass the result.



(RUN THE TESTS!)

So that Room doesn't need to depend at all on CarpetQuote.






October 8, 2017

Learn TDD with Codemanship

Manual Refactoring : Extract Method



Another commonly used refactoring is Extract Method. We may wish to extract a block of code or an expression that's repeated multiple times into a shared method, for example. Or break down a long method into a composed method, or document code by using a method name to explain what it does.



In this example, I have some test set-up code that's repeated three times. I'd like to extract this into a shared method which I can parameterise to make it reusable in all three instances.

This example is fairly straightforward. I just need to cut the repeated code from the first test and replace it with a call to a new method that I've yet to declare.



Then I declare the method create_quote(), and paste in the code I cut from the test. Because the object carpet_quote is referenced further down (in the test assertion), it needs to be a return value of this new method.



NOW RUN THE TESTS!

Now I'm free to introduce parameters for price per square metre and room width and length to make it reusable in the other tests.

In a slightly more nuanced example, the code we want to extract into a method references variables that are declared outside of that code.



Here, we'll need to introduce these parameters from the start. Again, cut the code we want to extract and replace it with a method call, this time passing in those variables.



Now declare the new method - with parameters - and paste in the extracted code.



AND RUN THE TESTS!

So, the process is quite straightforward.

1. First, cut the code you want to extract into a new method. Note that this code must executable in its own right - a statement, a block of code or an expression.

2. Identify any inputs - that is, references to variables declared before the extracted code.

3. Replace with a call to the new method, complete with any required input parameters.

4. Declare the new method and paste in the extracted code. If a variable is referenced after the extracted code, it will need to be made a return value.

5. Run your tests!

You've probably figured out that this won't work if there's more than one return value, so that would be a pre-condition to the Extract Method refactoring. In those cases, we can find other ways to act on a variable without passing it back (e.g., make it a field, or pass it by reference if the language allows that).




Learn TDD with Codemanship

Manual Refactoring : Introduce Parameter


Oftentimes I find myself extracting methods to remove duplication that start with hardcoded literals that need to be parameterised to make the new method reusable.

To introduce a parameter we need to do it in a single step, before we can run the tests again.



In this simple example, I want to parameterise the newly extracted factory method create_quote so that I can reuse it on the other two tests.

First, add a parameter to the method signature for the price per square metre.



Then cut the literal value for price inside the method and replace it with a reference to the new parameter.



Now paste the literal value we replaced as a parameter value in the method call. (If the method is being called in multiple places, you'll need to repeat this everywhere it's called.)



NOW RUN THE TESTS!

Rinse and repeat for room width and length, running the tests after completing each refactoring.



For an extra flourish, let's inline the local variable carpet_quote inside that method to simplify.



AND RUN THE TESTS!

Now we can reuse this method in the other two tests.






Learn TDD with Codemanship

Manual Refactoring : Inlining Complex Methods


Yesterday I posted about manually inlining variables and methods with a couple of simple examples. But inlining methods in particular is not always that straightforward, so today let's consider a beefier example.

What about inlining methods that contain more than a single expression? And what if any parameter values being passed in aren't local variables in the calling code, but more complex expressions themselves?

Well, actually, that's still pretty straightforward. Consider this example:



I want to inline the method calculate_price(). Again, the first step is to replace references to parameter names inside that method with the parameter values being passed in.



NOW RUN THE TESTS!

Then we can paste the code inside that method in place of the call to it.



AND RUN THE TESTS!

And then finally - because calculate_price() is no longer being used - we can delete the method.



DON'T FORGET TO RUN THE TESTS!!

Now let's think about situations where a method might be called in multiple places. How would we inline that?

Frankly, I probably wouldn't. If a method's being called in multiple places, that suggests that inlining it would introduce duplication. So, for me, a precondition to inlining methods is that they're only called in one place.

But if you absolutely must, then the only real variation in the process would be that you might need to match up the parameter values multiple times, if each caller is calling them something different.






October 7, 2017

Learn TDD with Codemanship

Manual Refactoring : Inlining Variables & Methods



My fourth post on manual refactoring is about inlining code. Essentially, this means replacing a reference to something with the implementation of the thing itself.

Fot example, I can simplify this Python code by inlining the result variable.



To do this, simply copy the expression assigned to the variable in its declaration and paste it over the reference(s) to the variable.



RUN THE TESTS!

Then delete the original declaration.



NOW RUN THE TESTS!



We can also inline the method sum_previous to simplify things further.

If I were to paste the body of this method in place where it's currently being called, the code will break because the variable being passed in as a parameter value is called index, but the method parameter is called . So first I need to rename the parameter.



AND RUN THE TESTS!

Next, paste the body of the method we want to inline where it's being called.



THEN RUN THE TESTS!

And now that sum_previous isn't being called from anywhere, we can delete it.



DON'T FORGET TO RUN THE TESTS!



October 3, 2017

Learn TDD with Codemanship

Manual Refactoring : Introduce Field

Next in my series on manual refactoring is a variation on Introduce Local Variable, where we want the result of an expression to be shared among methods: Introduce Field.

Consider an example where we have an object creation expression repeated in 3 unit tests.



I want to refactor this code so that the knowledge of object creation is in just one place, and because all 3 tests need to access the result, it'll need to be a field.

Like when I introduced a local variable, I'll start by cutting the whole expression and replacing it with a reference to the field-to-be.



Then I can declare the field and paste the creation expression as its initial value.



NOW RUN THE TESTS!

Next, I replace the other 2 duplicated expressions with a reference to the new field.



AND RUN THE TESTS AGAIN!

Of course, there's some wiggle room here. I could also have initialised the field in a constructor.



Or I could have initialised it using a set-up method.



But I went for the simplest solution, which works fine in this situation because:

a. Fibonacci is stateless, and its set-up is simple, and...

b. JUnit instantiates the fixture for every test anyway