Home > Articles > Programming > Ruby

A Refactoring Example in Ruby

  • Print
  • + Share This
  • 💬 Discuss

This chapter is from the book

In this chapter from Refactoring in Ruby, the authors share a quick example of refactoring to show how you can identify problems in code and systematically clean it up.

Rather than start with a lot of explanation, we’ll begin with a quick example of refactoring to show how you can identify problems in code and systematically clean them up. We’ll work “at speed” so you can get the feel of a real session. In later chapters, we’ll touch on theory, provide deeper dives into problems and how you fix them, and explore moderately large examples that you can practice on.

Sparkline Script

Let’s take a look at a little Ruby script Kevin wrote a while back. The script generates a sparkline (a small graph used to display trends, without detail) and does it by generating an SVG document to describe the graphic. (See Figure 1.1.)

The original script was written quickly to display a single sparkline to demonstrate the trends that occur when tossing a coin. It was never intended to live beyond that single use, but then someone asked Kevin to generalize it so that the code could be used to create other sparklines and other SVG documents. The code needs to become more reusable and maintainable, which means we’d better get it into shape.

Here’s the original code:


NUMBER_OF_TOSSES = 1000
BORDER_WIDTH = 50

def toss

5

   2 * (rand(2)*2 - 1)
end

def values(n)
  a = [0]

10

  n.times { a << (toss + a[-1]) }
  a
end

def spark(centre_x, centre_y, value)

15

   "<rect x=\"#{centre_x-2}\" y=\"#{centre_y-2}\"
    width=\"4\" height=\"4\"
    fill=\"red\" stroke=\"none\" stroke-width=\"0\" />

  <text x=\"#{centre_x+6}\" y=\"#{centre_y+4}\"
    font-family=\"Verdana\" font-size=\"9\"

20

    fill=\"red\" >#{value}</text>"
end

$tosses = values(NUMBER_OF_TOSSES)
points = []

25

 $tosses.each_index { |i| points << "#{i},#{200-$tosses[i]}" }

data = "<svg xmlns=\"http://www.w3.org/2000/svg\"
     xmlns:xlink=\"http://www.w3.org/1999/xlink\" >

  <!-- x-axis -->

30

  <line x1=\"0\" y1=\"200\" x2=\"#{NUMBER_OF_TOSSES}\" y2=\"200\"
            stroke=\"#999\" stroke-width=\"1\" />
  <polyline fill=\"none\" stroke=\"#333\" stroke-width=\"1\"
    points = \"#{points.join(' ')}\" />
  #{spark(NUMBER_OF_TOSSES-1, 200-$tosses[-1], $tosses[-1])}

35

</svg>"

puts "Content-Type: image/svg+xml
Content-Length: #{data.length}

40

 #{data}"

Forty lines of code, and what a mess! Before we dive in and change things, take a moment to review the script. Which aspects of it strike you as convoluted, or unreadable, or even unmaintainable? Part II, “Code Smells,” of this book lists over forty common code problems: Each kind of problem is known as a code smell, and each has very specific characteristics, consequences, and remedies. For the purposes of this quick refactoring demonstration, we’ll use the names of these smells (so that you can cross-reference with Part II, “Code Smells,” if you wish), but otherwise we just want to get on with fixing the code. Here are the more obvious problems we noticed in the code:

  • Comments: There’s a comment in the SVG document (line 29). As a comment in the SVG output that’s not a bad thing, because the SVG is quite opaque. But it also serves to comment the Ruby script, which suggests that the string is too complex.
  • Inconsistent Style: Part of the SVG document is broken out into a separate method (line 34), whereas most is built inline in the data string.
  • Long Parameter List: Strictly speaking, the list of properties of the XML elements aren’t Ruby parameters. But they are long lists, and we feel sure they will cause problems later.
  • Uncommunicative Name: The code uses data as the name of the SVG document, i as an iterator index (line 25), a as the name of an array (line 9), and n as the number of array elements (line 8).
  • Dead Code: The constant BORDER_WIDTH (line 2) is unused.
  • Greedy Method: toss tosses a coin and also scales it to be –2 or +2.
  • Derived Value: Most of the numbers representing SVG coordinates and shape sizes could probably be derived from the number of tosses and the sparkline’s max and min values.
  • Duplicated Code: The text markers for the start and end tags of XML elements are repeated throughout the code; the calculation 200-tosses[x] is repeated (lines 25, 34).
  • Data Clump: The SVG components’ parameters include several x-y pairs that represent points on the display canvas (lines 15, 18, 30). Some have further parameters that go to make up a rectangle (lines 16, 30). Strictly, these are parameters to SVG elements, and this is therefore a problem in the definition of SVG.
  • Global Variable: Why is tosses a global variable at all?
  • Utility Function: One might argue that all of the methods here (lines 4, 8, 14) are Utility Functions.
  • Greedy Module: The script isn’t a class, as such, but it does have multiple responsibilities: Some of the script deals with tossing coins, some deals with drawing pictures, and some wraps the SVG document in an HTTP message.
  • Divergent Change: The data string (lines 27–35) is probably going to need to be different for almost every imaginable variation on this script.
  • Reinvented Wheel: There are already Ruby libraries for manipulating XML elements, and even for creating SVG documents.

Which should we address first? When faced with a long to-do list of code smells it’s easy to feel a little intimidated. It’s important to remember at this stage that we can’t fix everything in one sitting; we’ll have to proceed in small, safe steps. We also want to avoid planning too far ahead—the code will change with every step, and right now it would be a futile waste of energy to attempt to visualize what the code might be like even a few minutes from now.

So in the next few sections we’re simply going to address the smells that strike us as “next” on the to-do list, without regard to what “next” might mean, or to what will happen after that. It is entirely likely that you would address the smells in a different order, and that’s just fine; experience suggests that we’re likely to finish up at approximately the same place later.

First, let’s tidy up a little.

  • + Share This
  • 🔖 Save To Your Account
Refactoring in Ruby (Rough Cuts)

This chapter is from the book

Refactoring in Ruby (Rough Cuts)

Discussions

comments powered by Disqus