Even More Refactoring Techniques

Introduction

Well here we are again to learn some more refactoring techniques. If you want to recap the other techniques then go read my previous post, otherwise let's just crack on...

Remove Redundancy

This isn't an explicit technique, more a grouping of techniques.

The principle idea being that: code evolves, and as it evolves you may find techniques you previously implemented (as part of an earlier refactoring) have since become redundant.

Imagine you implemented the "Introduce Named Parameter" technique (passing a hash with named properties as a single argument instead of multiple unidentified arguments).

Now, after some other refactorings have taken place, you discover the method originally refactored is no longer as complex and so your argument hash refactor has been reduced down to just a single named property.

In this particular scenario you should remove the named parameter and simply pass a single argument instead.

This principle applies with other refactoring techniques.

Imagine an earlier refactoring included implementing a default parameter value for a method call. As your code evolves, if you discover you now only ever call the method with an argument then the default value becomes redundant and makes the code more complex than it needs to be by providing a default value. So just remove the redundant code.

Dynamic Method Definition

Sometimes defining multiple methods can be wasteful when functionally they carry out similar steps.

For example, imagine we had the following code...

def failure do 
  self.result = "failure" 
end 

def success do 
  self.result = "success" 
end 

def error do 
  self.result = "error" 
end 

Notice how the functions are structurally identical. They simply set a result property to have a value This can be refactored using Ruby's define_method method (which let's you create methods dynamically at run time)...

[:failure, :success, :error].each do |method| 
  define_method method do 
    self.result = method.to_s 
  end 
end 

Note: you could also abstract this code into a more reusable (and easier to maintain) function like so...

def dynamic_methods(*method_names, &block) 
  method_names.each do |method_name| 
    define_method method_name do 
      instance_exec(method_name, &block)
    end 
  end 
end 

You can also use this technique to help ease creating properties on an object. For example, I used this technique in my MVCP blog post to dynamically create instance variables...

require 'app/presenters/base' 
require 'app/models/person' 

class Presenters::Person < Presenters::Base 
  attr_reader :run, :name, :age 

  def initialize 
    @run = true 

    model = Person.new('Mark', '99') 
    prepare_view_data({ :name => model.name, :age => model.age }) 
  end 
end 

module Presenters 
  class Base 
    attr_accessor :model 

    def prepare_view_data hash 
      hash.each do |name, value| 
        instance_variable_set("@#{name}", value) 
      end 
    end 
  end 
end 

Extract Class

This is a pretty standard technique which helps ensure your objects abide by the SRP (Single Responsibility Principle).

If you find your classes are doing too much then simply create a new class and move the relevant fields and methods over one by one (while running the tests as you go to ensure all code continues working as expected).

Doing so you'll end up with two small, focused and clean classes which are easier to manage.

Hide Delegate

This technique focuses on the principle of object encapsulation. Specifically decoupling two or more objects by reducing the context the objects have of each other.

The following code demonstrates the idea...

module Bar
  def display
    puts "Bar Stuff"
  end
end

module Baz
  def display
    puts "Baz Stuff"
  end
end

class Foo
  include Bar

  def do_something
    display
  end
end

foo = Foo.new
foo.do_something

...as you can see, the user only needs to rely on the interface having a do_something method.

The implementation details of do_somthing (in this case the delegation off to another method) are hidden.

If we changed include Bar for include Baz, or maybe we don't mixin a module at all and just write some code inside of do_something, it doesn't matter because the public interface is set as far as the user is concerned.

Replace Array with Object

The motivation for this technique is to convert a simple data container which holds multiple data types into an object with clear and descriptive identifiers.

This principle helps to present your complex data into a more sensible format (I demonstrated this in a previous post on object-oriented design). This technique also makes the data interaction more maintainable by providing an easier and understandable interface to the data.

Here is an example where we're violating the principle of a clean data interaction...

class Foo 
  attr_reader :data 

  def initialize(data) 
    @data = data 
  end 

  def do_something 
    data.each do |item| 
      puts item[0] 
      puts item[1] 
      puts '---' 
    end 
  end 
end 

obj = Foo.new([[10, 25],[3, 9],[41, 7]]) 
obj.do_something 

Notice in the first example how our code has far too much knowledge (context) about the object it's interacting with. It knows that the Array index zero holds an X coordinate and the Array index one holds a Y coordinate.

If that format was to change (let's say the X and Y swap places) then that would cause our code to break in unexpected ways.

But now take a look at the following example which works around this issue by converting our complex data structure into a cleaner data format...

class Foo 
  attr_reader :new_data 

  def initialize(data) 
    @new_data = transform(data) 
  end 

  def do_something 
    new_data.each do |item| 
      # now we are able to reference easily understandable 
      # property names (rather than item[0], item[1]) 
      puts item.coord_x 
      puts item.coord_y 
      puts '---' 
    end 
  end 

  Transform = Struct.new(:coord_x, :coord_y) 

  def transform(data) 
    data.collect { |item| Transform.new(item[0], item[1]) } 
  end 
end 

obj = Foo.new([[10, 25],[3, 9],[41, 7]]) 
obj.do_something 

...here we convert the Array into an object and instead can more easily and safely reference the data we're interested in via recognisable property identifiers. This doesn't mean if the data source changes that we'll totally avoid all problems but it'll be clearer where the problem is arising.

Replace Conditional with Polymorphism

This is one of the most useful refactoring techniques available to you, and there are two ways it can help:

  1. It removes the code smell of conditional logic
  2. It demonstrates perfectly the principle of object-oriented design

The following example shows the typical procedural attempt to handle different scenarios based on the data object type being passed...

class Foo
  def initialize(data)
    @data = data
  end

  def do_something
    if @data.class == Bar
      puts "Bar!"
    elsif @data.class == Baz
      puts "Baz!"
    elsif @data.class == Qux
      puts "Qux!"
    end
  end
end

class Bar; end
class Baz; end
class Qux; end

foo_bar = Foo.new(Bar.new)
foo_bar.do_something

foo_baz = Foo.new(Baz.new)
foo_baz.do_something

foo_qux = Foo.new(Qux.new)
foo_qux.do_something

...as you can see, if we have a new Class type we need to go back and to modify the Foo base class. This violates the OCP (Open/Closed Principle) which states a file should be open for extension but closed for modification.

For us to abide by OCP we can use polymorphism and a trusted interface/duck typing to solve the problem...

class Foo
  def initialize(data)
    @data = data
  end

  def do_something
    @data.identifier
  end
end

class Bar
  def identifier
    puts "#{self.class}!"
  end
end

class Baz
  def identifier
    puts "#{self.class}!"
  end
end

class Qux
  def identifier
    puts "#{self.class}!"
  end
end

foo_bar = Foo.new(Bar.new)
foo_bar.do_something

foo_baz = Foo.new(Baz.new)
foo_baz.do_something

foo_qux = Foo.new(Qux.new)
foo_qux.do_something

Notice we have removed the need for a conditional and just sent the message to the relevant object to be handled. Much cleaner and easier to maintain and scale.

Decompose Conditional

Not all conditional statements can be avoided through the use of polymorphism. In those cases you can simplify the conditional logic (and the subsequent statements) by extracting them into external methods.

Here is a simple example...

if date < SUMMER_START || date > SUMMER_END 
  charge = # some complex calculation if it's winter 
else 
  charge = # some other complex calculation if it's summer 
end 

...which we can refactor like so...

if not_summer(date) 
  charge = winter_charge 
else 
  charge = summer_charge 
end 

...much better.

Introduce Null Object

The motivation behind this technique is to avoid using a conditional whose purpose is to check whether a property exists or not before using it.

Here is a simple example of what we want to avoid...

class Post
  attr_reader :id

  def initialize id
    @id        = id
    @published = false
  end

  def self.find_and_publish id
    # Simulated database operation
    post = Posts.find { |post| post.id == id }
    post.publish unless post.nil?
  end

  def publish
    puts @published = true
  end
end

Posts = [Post.new(1), Post.new(2)]

Post.find_and_publish(0) # displays nothing
Post.find_and_publish(1) # displays true

...in the above example we check whether post is nil or not. If it isn't nil then we call the publish method, otherwise we don't do anything.

This is kind of ugly.

The following code demonstrates how we can avoid that problem by introducing the concept of having an object to handle null scenarios (it's the same principle of using duck typing/trusted interfaces/polymorphism)...

class Post
  attr_reader :id

  def initialize id
    @id        = id
    @published = false
  end

  def self.find_and_publish id
    # Simulated database operation
    post = Posts.find { |post| post.id == id } || NullPost.new
    post.publish
  end

  def publish
    puts @published = true
  end
end

class NullPost
  def publish
    # noop
  end
end

Posts = [Post.new(1), Post.new(2)]

Post.find_and_publish(0) # displays nothing
Post.find_and_publish(1) # displays true

...as you can see, effectively we have the same code with the exception that we no longer check for nil? in the second example and instead we rely on another object NullPost implementing the same interface but returns a null related value.

This way we're using objects to handle our logic. Yes, we end up with more code (one extra Class) but ultimately this is more maintainable and understandable than lots of inline logic.

Conclusion

Again, there are still many different refactoring techniques that I've not included. But hopefully you've found this second instalment just as useful as the first, I'll update the post or start anew with more techniques soon.


Links