Tag Archives: Refactoring

Building a Bridge a.k.a Parallel Changes

Several days ago, we had a coding dojo at Digital Natives: we wanted to reimplement a method without changing its purpose. To do this, we used a technique called building a bridge, which is also known as parallel change. The technique itself is quite simple, and in this post I’m going to show you how to use it and why.

For example, I have this piece of code, which is presumably part of a larger code base:

class Foo
  # ...
  def count(first, last)
    counter = 0
    first.upto(last) do |i|
      counter += 1 if prime?(i)
    end
    counter
  end
 
  private
  def prime?(value)
    [2,3,5,7,11,13,17,19].include?(value)
  end
 
  # ...
end

Maybe it is not working very well – its limitation is obvious -, but it is more than enough for now. Let’s say that we decide to change the prime? method. The same method, the same purpose, only the algorithm will be different. So we create a new method called prime_next? and implement that method either with TDD or in a regular fashion. After it is done, we’ll have something like this:

class Foo
  # ...
  private
  def prime?(value)
    [2,3,5,7,11,13,17,19].include?(value)
  end
 
  public
  def prime_next?(value)
    # a fancy algorithm probably based on Fermat's primality test
  end
  # ...
end

When our tests or gut feelings indicate that the new method is complete, we replace the method names, so prime? will be come prime_next? and prime_next? will become prime? (hint: it is easier and safer to switch the actual method names than to change the references. For example, in ruby we don’t have a fancy IDE with the “rename method...” functionality so it would take some time until all the references are changed, not mentioning the expressions evaluated at runtime).

We run some more tests – or trust our feelings – so that we see whether the new algorithm is working with the rest of the code base. If everything is up and running, we clean up and deliver:

class Foo
  # ...
  def count(first, last)
    counter = 0
    first.upto(last) do |i|
      counter += 1 if prime?(i)
    end
    counter
  end
 
  private
  def prime?(value)
    # a fancy algorithm probably based on Fermat's primality test
  end
  # ...
end

This was the “how”, but it seems a lot of unnecessary work for a change, doesn’t it? It may seem like that at first, but I prefer to do it, because it ensures that while I’m working on my changes, I can still work with the rest of the team, and that the code I’m working on is well tested. Now, let’s see the “why”.

Read more »

VN:F [1.9.13_1145]
Rating: 0.0/10 (0 votes cast)

Code Review During Retrospective

Most of the retrospectives I’ve kept or participated in were about agile approaches (for example communication with the Product Owner) and organisation-related changes, but not everybody is into these. Most software engineers and craftsmen aren’t that interested in how to deliver faster, or how to communicate better, they are interested in how to be better at their profession: programming.

Usually, software craftsmen are interested in new technologies and improving their programming skills. The easiest way to gain knowledge and improve skills is to learn from each other, see how others are programming, what kind of tricks they are using, what problems they have, and how they solve them. Last but not least, when you read other people’s code, you will have more information about what is going on in the project(s) you are working in.

My proposal is to do code review during your retrospective meetings.

Usually, this kind of retrospective has the following goals:

  • learn new programming techniques
  • find quality improvement ideas
  • find coding behavioural patterns which need to be kept
  • find coding behavioural patterns which need to be changed

It is really important to keep these goals in mind, because the retrospective is not a real code review session where the goal is to find mistakes and correct them. It is about finding those things which can make you a better craftsman in the long run.

Read more »

VN:F [1.9.13_1145]
Rating: 8.5/10 (2 votes cast)

Weekly – CW12

I usually exchange interesting articles, presentations and links with my friends in many different ways. I thought it would be better to use one simple way, so I’m going start a weekly series where I post articles, presentations and links I read during the week and find interesting. So here is the collection for calendar week 12, 2011:

  • Google was always famous for innovation. Patrick Copeland presented the eXtreme innovation approach – used by Google -, in his Keynote at QCon 2010. Pretotyping seems to be a good way for implementing only those ideas that really matter. It also helps not to spend time and money on ideas that people will hardly ever or never going to use.
  • My average e-mail inflow was approximately 90 e-mails per day and I read about 5 of them. I was happy to read an article about how to write e-mails effectively. If everyone who wrote to me, had read this article and had written mails as the author had suggests, I might have had the chance to read all of them.
  • Last but not least, here comes a great post about handling waste properly in lean systems. As usual, waste elimination is not black or white. A good lean/Kanban team (or team leader) should be able to recognize the nature of the waste, and eliminate what is really unnecessary.
VN:F [1.9.13_1145]
Rating: 0.0/10 (0 votes cast)

The Currency Format Kata

A couple of days ago, I needed a function which is capable of printing out a certain amount in Hungarian currency format. At that time I found the implementation of this function challenging, so I decided to implement it on my own, without googling for an existing solution. It was fun – as I expected – and I also realized that it would be a great Coding Kata exercise, because:

  • The exercise is quite simple
  • It can be finished in a short period of time
  • There are multiple choices on how to do it

The definition of the exercise:

Implement a functionality which shall receive a double number as string input. It shall format its input according to the Hungarian currency format, and it shall return the formatted value as string. In this particular format, the thousands are separated with spaces, and the decimal mark is a comma. You can assume that the input is always in the right format.

For example:

  • 100 -> 100
  • 1000 -> 1 000
  • 1000000.01 -> 1 000 000,01

I did the pilot Kata with my friend and colleague Tamás Győrfi in a pair programming session with TDD. we had a great time, and learnt new things even though we were unable to finish our implementation due to our time limit on the kata session. The following code demonstrates how far we got:

public class HungarianCurrencyFormatter {
    public String format(String number) {
        if (isInteger(number)) {
            return insertThousandSeparator(number);
        } else {
            String integerValueOfTheNumber = number.split("\\.")[0];
            StringBuilder formattedNumber = new StringBuilder();
            formattedNumber.append(insertThousandSeparator(integerValueOfTheNumber));
            formattedNumber.append(",");
            formattedNumber.append(number.split("\\.")[1]);
            return formattedNumber.toString();
        }
    }
 
    private boolean isInteger(String number) {
        return !number.contains(".");
    }
 
    private String insertThousandSeparator(String number) {
        final StringBuilder formattedNumber = new StringBuilder(number);
        for (int i = 1; i <= (number.length() / 3); i++) {
            int position = number.length() - 3 * i;
            if (position != 0) {
                formattedNumber.insert(position, " ");
            }
        }
        return formattedNumber.toString();
    }
}

The code above could use some refactoring, because there are duplications in the code such as calling the String#split() method twice, and the inserThousandSeparator method is not self-explanatory.

I tried a different way using the NumberFormat.getCurrencyInstance(Locale) method. The returned formatted String had some extras in it like an “ Ft” suffix and a strange thousand separator:

public class HungarianCurrencyFormatter {
    private static Locale HUN = new Locale("hu", "HU");
 
    public String format(String number) {
        NumberFormat currencyFormatter =
            NumberFormat.getCurrencyInstance(HUN);
 
        String currencyFormattedNumber =
            currencyFormatter.format(Double.parseDouble(number));
 
        String formattedNumberWithProperGroupSeparator =
            replaceLocaleSpecificGroupSeparatorWithSpace(currencyFormattedNumber);
 
        return removeFtSuffix(formattedNumberWithProperGroupSeparator);
    }
 
    private String removeFtSuffix(String number) {
        return number.substring(0, number.indexOf(" Ft"));
    }
 
    private String replaceLocaleSpecificGroupSeparatorWithSpace(String number) {
        char separator = DecimalFormatSymbols.getInstance(HUN).getGroupingSeparator();
        return number.replace(separator, ' ');
    }
}

The full source code is available on github.

The extras made the code a bit complicated, and I don’t feel that the second solution is much better than the first one. So I’ll keep exercising, maybe I found one which is much better than the versions above.

If the exercise is no longer challenging for you, you can do the following:

  • Accept any kind of string, and do proper error handling
  • Do the Kata with different constraints, like
VN:F [1.9.13_1145]
Rating: 10.0/10 (2 votes cast)

The Refactoring only Constraint

Doing code kata is more fun with constraints. Once I heard about a very impressive one, but I was unable to find anything about it, so I decided to investigate and write about it on my own.

The constraint is simple: only refactoring can be done in the production code, in other words: any kind of new functionality shall be tested and implemented in test code and refactored to the production code.

Until now, I was unable to find any reasonable argument that favours this methodology in a production environment, however, doing code kata with this constraint really improves…

  • …knowledge about the refactoring capabilities of the tools being used
  • …the refactoring experience
  • …communication skills – if done in pairs
  • …craftsmanship and patience

My recommended steps for the first try:

  1. Find an easy code kata
  2. Write a small piece of functionality with TDD, but keep it in the test case
  3. Move that small piece of functionality to production code
  4. Write another small piece of functionality with TDD, but still keep the real and the test code in the test case
  5. If necessary, refactor the production code so that it can accept the new functionality
  6. Write an integration test case which tells how the production code shall behave after moving the new piece of code to the production
  7. Move that small piece of functionality to the production code
  8. Refactor the test cases, remove redundancy – for example, the test cases of steps 2 and 4
  9. Refactor the code and keep encapsulation in mind
  10. Continue with step 4, until everything is implemented

Of course, you can choose different ways, but pay attention to the importance of the integration test case. It makes sure that nothing is broken after the move operations.

While doing the refactoring, try to…

  • …use only the refactoring tools and assistance features of your IDE – this will help you learn the capabilities of your tool(s)
  • …use only documented refactoring techniques (Fowler – Refactoring, Kerievsky – Refactoring to Patterns) – this will help you gain lexical knowledge
  • …avoid adding new [helper] lines to the code, even if you know that you will remove them later – there is a good chance that these lines will remain in the code after all

As an example, I did Roy Osherove’s String Sum exercise, with some small changes:

  • The input is always one line and always valid
  • The separator is ‘,’

I’m using eclipse, and focusing on the most important parts of work, meaning that I’m covering only one angle in the following example: return the summary of an input such as “2,3,5″

The first functionality:

    @Test
    public void shouldSumAnArrayOfIntegers() {
        assertEquals(10, sumNumbers(new int[]{2, 3, 5}));
    }
 
    int sumNumbers(int[] numbers) {
        int sum = 0;
        for (int number : numbers) {
            sum += number;
        }
        return sum;
    }

Now I’m creating a stringSum private field with the type StringSum, and moving the sumNumbers there with right click on the method -> Refactor… -> Move…

Now comes the parsing:

    @Test
    public void shouldConvertStringToIntArray() {
        assertArrayEquals(new int[]{2, 3, 5}, convert("2,3,5"));
    }
 
    int[] convert(String string) {
        String[] items = string.split(",");
        int[] numbers = new int[items.length];
        for (int i = 0; i < items.length; i++) {
            numbers[i] = Integer.parseInt(items[i]);
        }
        return numbers;
    }

So far everything is green, now comes the integration test case:

    @Test
    public void shouldPerformTheSumOnTheInputString() {
        assertEquals(10, stringSum.sum("2,3,5"));
    }

The sum() method shows up, makes the test code red (compilation failure), but with the quick fix (CTRL + 1) on the method, I can create it, and now it’s just the test assertion itself that fails. In order to finish, the convert() method is required, but it is still in the test code, and until I have a green bar, I’m not really allowed to change the code base. I am ignoring the integration test case for a minute to have a green bar, and moving the convert() method to the production code. After having the test case on board again, the bar is red, but using the content assist (CTRL + space) I’m inserting the convert() and sumNumber() calls:

    public int sum(String string) {
    	return sumNumbers(convert(string));
    }

Now everything is green, but the code is a bit ugly. The test code does not need the shouldSumAnArrayOfIntegers() and shouldConvertStringToIntArray() test cases, so I’m removing them, and making every method in the StringSum class private except the sum().

After using Refactor -> Inline…, and moving around some lines (ALT + up/down arrow) the StringSum has only one method, which looks like this:

    public int sum(String string) {
        int sum = 0;
        String[] items = string.split(",");
        for (int i = 0; i < items.length; i++) {
            sum += Integer.parseInt(items[i]);
        }
        return sum;
    }

The example above is very simple. After adding more functionality – like supporting more separators and having proper error handling -, the task became harder. It took me hours to finish it, but on the bright side, I’ve learnt new refactoring techniques.

VN:F [1.9.13_1145]
Rating: 0.0/10 (0 votes cast)