Service Oriented Architecture

We just finished up our penultimate project at gSchool, an application built using a Service Oriented Architecture (SOA). The project ideas were generated in small groups, and focused a the theme of health and wellness. Once the ideas were decided, our teacher Jeff Casimir generated a randomized list of the students, and we drafted projects.

I went first, and selected an idea that was close to my heart: an app to help home vegetable gardeners plan out their beds for the season. Having spent several years as an organic farm worker, garden company owner, and home garden hobbyist, I was excited to use this app. Every spring, before the ground warms up, my partner and I sit down with a pencel and graph paper to plan out the different beds in my garden: what will go where, how it will be spaced, when it will be harvested, and what we’ll plant there afterwards.

I had big hopes for our project, dubbed Planting Season (code). A user would have several gardens, each with many beds. The app would keep track of her plans from month to month, and she would be able to click on a month to see what she’d planned to plant there, which spots were empty, and which plants were ready for harvest. It would find her USDA Hardiness Zone and suggest plants well-suited to grow there. If there was impending frost, or a bed needed water, it would send her a text message, email, or a Google voice message.

Ah, the dreams of the young and inexperienced…

Planting Season’s Structure

Two of my group members had just completed FooFoBerry, an SOA app that exposed and consumed APIs from various project management tools, such as GitHub, Travis, and Code Climate. With their experience setting up services, I figured that getting the project up and running would be a breeze. In practice, it was more challenging than I expected.

I’m a big fan of a “lean startup” / “minimum viable product” approach in my projects. I like to get a simple product up as fast as possible, then expand on it, adding value. Given that we had three weeks, we planned three iterations for Planting Season:

Week 1 - Put up a landing page where users could enter their email to be notified when the app was ready.

Week 2 - Change the landing page to allow users to sign up or sign in, then send them to a dashboard page, where they could add and remove plants from a single garden.

Week 3 - Add functionality for multiple beds, text and email weather notifications, and a timeline for the garden through the year.

With this functionality in mind, we decided to build the following services:

  1. Landing Page App
  2. User Authentication App
  3. Client-Facing Dashboard (“Coordinator”) App
  4. RESTful JSON API with Bed and Plant data
  5. A gem to facilitate communication between the Dashboard and the API
  6. External Services - Weather and Geolocation APIs

Here’s how the project ended up looking in the end:


A web application diagram

Project Management & Workflow

Over the course of gSchool, I’ve been learning a lot about the social aspect programming. I worked solo in my ealiest projects, then graduated to pairing, and later went on to work in groups of three or four. In the beginning, I was just doing my own thing, but as I started getting into the larger groups, project management became a key skill. I’ve found myself in the role of Project Manager in many successive projects. The first couple of times were messy: I didn’t make enough space for people to express how they were feeling at our standups, our iteration planning was nonexistent, and I didn’t understand how to use Pivotal Tracker at all. As I learned from these experiences, I began to firmly value things like daily standups with technical and emotional checkins, persona and wireframing UX design, iteration planning, proper use of Pivotal Tracker, consistent version control practices, and continuous integration.

We started Planting Season with the best of intentions, but this time around, it proved harder to practice these strategies than I hoped. At our checkins, it was clear that we were all feeling unfocused and pulled in different directions by job applications and interviews, burnout, and Jeff’s absence after his wife gave birth to their latest addition to the family. We planned our iterations, but we didn’t meet the first one because a couple of us were out of town or at interviews. Soon we fell behind. On top of that, Pivotal Tracker stopped letting us edit our stories, because our free trial was expired. The Travis CI specs wouldn’t pass, because the specs couldn’t talk to the API (more on this in a minute). Of all the agile practices we hoped to use, only the UX planning went well.

Nginx, Passenger, and VPS Woes

Our troubles were compounded by the difficulty of getting our services to talk to each other. Our plan was to run Foreman locally, booting our apps onto different ports on localhost, then use Nginx to route everything through different namespaced routes on port 8080. Getting Foreman up and running was easy enough, but Nginx was a bear to configure. Meanwhile, getting Phusion Passenger and Nginx to play nicely on our VPS was also a struggle. It turns out that you have to install Passenger first, then add Nginx as an extension to it. If you install Nginx first, you end up with two conflicting versions. Additionally, one of our team mates was having RVM issues, and had to reinstall Ruby and all of his gems twice.

In the second week, we lost two solid days of of development time to the fight with the Nginx config file, Passenger, and RVM.

For posterity’s sake, here’s a snapshot of the nginx.conf file that finally worked for local production:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
worker_processes  1;

error_log  logs/error.log;
#pid        logs/nginx.pid;

events {
    worker_connections  1024;
}

http {
    include       mime.types;
    default_type  application/octet-stream;
    #passenger_root /usr/local/opt/passenger/libexec/lib/phusion_passenger/locations.ini;
    #passenger_ruby /usr/bin/ruby;

    server {
        listen       8080;
        server_name  localhost;

        # PLANTING SEASON

        # Landing Page
        location / {
          proxy_pass        http://127.0.0.1:3000;
          proxy_set_header  X-Real-IP  $remote_addr;
        }

        # Auth
        location /auth {
          proxy_pass        http://127.0.0.1:3001;
          proxy_set_header  X-Real-IP  $remote_addr;
        }

        # Coordinator
        location /dashboard {
          proxy_pass        http://127.0.0.1:3002;
          proxy_set_header  X-Real-IP  $remote_addr;
        }

        # API
        location /api {
          proxy_pass        http://127.0.0.1:3003;
          proxy_set_header  X-Real-IP  $remote_addr;
        }
   }
}

Testing Troubles

Once we had the services running on a single port, we figured everything would be gravy. We added the VCR gem to save the results of our API calls, and began testing. We covered the landing page, auth app, and our gem.

Then I started to dig in on some Capybara tests for the coordinator app, and had some major headaches. In order to view the dashboard, I needed to set a signed cookie to simulate the authorization app. Easy enough. All you have to do is use:

1
page.driver.browser.set_cookie 'user_id=1'

That is, unless you’re using Poltergeist for a JS driver. Then, you clearly use:

1
page.driver.set_cookie("user_id", 1)

But now, I had a different problem:

1
2
3
Failure/Error: within '#bed-functions' do
     Capybara::ElementNotFound:
       Unable to find css "#bed-functions"

Using the launchy gem and save_and_open_page, #bed-functions was there, all right. I could feel another configuration battle coming on as we headed into the third week.

Getting Back on the Horse

On Monday morning of the week the project was due, we had little to show for our two weeks of work. There were services, but they couldn’t talk to each other on the server. There were mock-ups, but the CSS wasn’t in the views yet. The tests were thin. We had a meeting with Jeff, and told him we planned to bolster the test coverage and solidify what we already had. His advice: “that would be a good idea if you had a thing, but right now you don’t. You might want to make a thing first.”

So we saddled up, and the cowboy coding began.

jQuery Sparkle

The next two days were actually pretty fun, if a little painful (coding without tests makes me uncomfortable). We got the CSS hooked up, integrated with the weather service, and added a healthy dose of JavaScript to the dashboard. Although we would have liked to explore Ember.js, we decided to stick to plain old jQuery in the interest of time. We quickly coded 275 lines of jQuery sparkle, and we had a thing! If you’re interested, go check out what we made!

My favorite part was adding the ability to click and drag on the garden bed squares for multi-selection. Here’s what that looks like:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
$(function() {
  window.isMouseDown = false;

  $('td').mousedown(function(e){
    window.isMouseDown = true;
    var element = $(e.currentTarget);
    var thisClass = $(this).attr("class");
    toggleSquare(thisClass, element);
    return false;
  });

   $('td').mouseenter(function(e){
    if (window.isMouseDown){
      var element = $(e.currentTarget);
      var thisClass = $(this).attr("class");
      toggleSquare(thisClass, element);
    }
    return false;
  });

  $(document).mouseup(function () {
    window.isMouseDown = false;
  });

});

If you want to see more of what we did with the JS, you can check out the code on GitHub.

Conclusion

From the beginning of gSchool, the SOA project was one of the things I was looking forward to the most. Although Jeff said he thought that it was probably “not a reasonable thing” to start a project from the beginning with SOA, I set out with the highest of expectations. But the problems along the way were numerous.

The job search was a distraction. But more troubling to me was the difficulty of testing. I’ve gotten really comfortable with using Travis CI, and driving my development with Pivotal Tracker user stories translated into Capybara feature tests. It was really hard to do with services. Between setting up Passenger and Nginx and the testing troubles, we lost a lot of time on configuration.

Reflecting on the experience, I’d say that SOA is probably a practice better left until it is needed. If I were in charge building a new app for a startup, I would still start by iterate on delivering business value with successive MVPs. When I began to feel the pain of scaling, then I would think about splitting off services.

However, it’s possible that I’ve only come to this conclusion because the process was so painful. There’s another way of looking at this.

Here’s a lesson that I’ve learned about programming again and again over the last six months:

1
2
3
The first time you do something it is hard and doesn't make any sense.
The second time you do it, it seems vaguely familiar, and makes a little bit of sense.
From then on, it makes sense and feels natural to do it.

If this is true (which it seems to be for me), then it might actually make sense to build apps using services from the get go, if you expect them to have to scale down the road. I’d love to nail down a workflow for getting Passenger, Foreman, and Nginx set up in less than 30 minutes. I’d also like to learn how to better test services and get them set up with continuous integration. I think that if I understood these things, SOA would be a tool I’d be a lot more likely to reach for.

If you have any tips about these two aspects of services, let’s talk! Tweet at me: @fluxusfrequency.

Building a Binary Search

A couple of days ago I had an interview with a big startup in Boulder. I was really excited about the possibility of working there, because they solve some really hard problems on a daily basis, and because I’ve heard great things about their office culture.

It was just supposed to be a quick interview, but it ended up taking a few hours. First, I met up with the CTO. It all started off innocent enough. We went to a restaurant on Pearl Street for lunch. He asked me about my background and skills, and I asked him about the company. We shared personal stories about the flood last summer. After hearing about the way he treats his workers (like adults), and some of the technical details of what they were working on, I was even more excited about the company. We headed went to a coffee shop for a drink, then went back to the office.

When we got there, he introduced me to the head of the team that the best fit for me if I worked there. The team lead, one of his coworkers and I headed back to the coffee shop. We chatted about Rails, Exercism, TDD, and Ember.js, and life at their company. Pretty soon, the CTO came in again. Seeing that we were still talking, he asked the team lead to take me back and “dig in technically” a little bit.

We went back to the office again, and found a conference room. He asked me to solve FizzBuzz in JavaScript. I fired up Jasmine-Node and built it with tests the whole way. That went fairly well, I thought. Then he asked me a couple of CSS questions. I answered them, and he said that I answered them better than many of the people he interviews.

Finally, we came to some algorithm questions. Although I’ve learned a lot at gSchool in the last five months, algorithms are something that I haven’t spent as much time on, yet. I’ve only had enough programming context to begin understanding them for the last two of those months. I’ve played around with them as much as I could, solving Linked List, the Luhn Algorithm, Nth Prime Factor, Sum of Squares, and Binary Search Tree. I’ve also had a go at writing the MD5 algorithm. But compared to someone with a four year CS degree, I haven’t had much time to get comfortable with them.

My interviewer drew an array on the board, and asked me how I would find a given element’s position within 2-3 queries. I thought about it, made a guess that it had to do with bit operators, and finally conceded that I had no idea. My heart sunk. Things were going so well. When I asked for the solution, he told me that it was to use a Binary Search. I got excited, because I had done a Binary Search Tree. But no, this was just plain old Binary Search.

Bitten that I didn’t know how to do Binary Search, I set out to build it on my own. This rest of this post is a walkthrough of how I solved it.

Research

First, I visited Wikipedia and read up on Binary Search. It was pretty easy to follow. So easy, in fact, I decided to lift some of the text verbatim and translate it into a set of tests to drive my implementation. Here’s what I found:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
Searching a sorted collection is a common task. A dictionary is a sorted list of
 word definitions. Given a word, one can find its definition. A telephone book
 is a sorted list of people's names, addresses, and telephone numbers. Knowing
 someone's name allows one to quickly find their telephone number and address.

If the list to be searched contains more than a few items (a dozen, say) a
binary search will require far fewer comparisons than a linear search, but it
imposes the requirement that the list be sorted.



In computer science, a binary search or half-interval search algorithm finds
the position of a specified input value (the search "key") within an array
sorted by key value.

In each step, the algorithm compares the search key value with the key value of
the middle element of the array.

If the keys match, then a matching element has been found and its index, or
position, is returned.

Otherwise, if the search key is less than the middle element's key, then the
algorithm repeats its action on the sub-array to the left of the middle element
or, if the search key is greater, on the sub-array to the right.

If the remaining array to be searched is empty, then the key cannot be found in
the array and a special "not found" indication is returned.

A binary search halves the number of items to check with each iteration, so
locating an item (or determining its absence) takes logarithmic time. A binary
search is a dichotomic divide and conquer search algorithm.

I decided to take this step-by-step description and turn each step into a test.

Setting Up the Binary Search Test & Class

The first step was to create an object that knew about “an array sorted by key value”, or as I like to call it, a list. I wrote a test:

1
2
3
4
5
6
7
8
class BinarySearchTest < MiniTest::Test

  def test_it_has_list_data
    binary = BinarySearch.new([1, 3, 4, 6, 8, 9, 11])
    assert_equal [1, 3, 4, 6, 8, 9, 11], binary.list
  end

end

And a class to make it pass:

1
2
3
4
5
6
7
8
class BinarySearch
  attr_reader :list

  def initialize(data)
    @list = data
  end

end

That was easy! Next, it needed to be able to check whether the list was actually sorted, so I wrote a test to make sure it would throw an error if it wasn’t:

1
2
3
4
5
def test_it_raises_error_for_unsorted_list
  assert_raises ArgumentError do
    BinarySearch.new([2, 1, 4, 3, 6])
  end
end

To make it pass, I implemented the following check: compare the input with itself in sorted form. Not that elegant, but it worked:

1
2
3
4
5
6
7
8
9
class BinarySearch
  attr_reader :list

  def initialize(data)
    raise ArgumentError unless data.sort == data
    @list = data
  end

end

According to the Wikipedia article, the algorithm was going to have to find “the key value of the middle element of the array”. So I wrote a test to make sure that it knew where the middle of its list was:

1
2
3
4
def test_it_finds_position_of_middle_item
  binary = BinarySearch.new([1, 3, 4, 6, 8, 9, 11])
  assert_equal 3, binary.middle
end

This was fairly trivial to solve:

1
2
3
4
5
6
7
8
9
10
class BinarySearch
  attr_reader :list

  # Code omitted

  def middle
    list.length/2
  end

end

With the class all set up, I turned my attention to the actual search algorithm itself.

Building the Search Algorithm

The goal was to search for the index of a specific element, so I wrote a test for a method that did that:

1
2
3
4
def test_it_finds_position_of_search_data
  binary = BinarySearch.new([1, 3, 4, 6, 8, 9, 11])
  assert_equal 5, binary.search_for(9)
end

Next came the hard part. I took the text from Wikipedia and broke it down into pseudo-code:

  • Get the search value
  • Find the middle element in the list
  • Check if they are equal
  • If they are equal, return the position of the middle element in the list
  • If they are not equal, branch:

If the search key is less than the middle index:

  • Cut the list we’re searching in half
  • Instantiate a new BinarySearch
  • Pass the first half of the list into it
  • Call the search method on the new object, with the same search key
  • Repeat until the keys are equal
  • If they’re never equal, raise “Not Found”

If the search key is more than the middle index:

  • Cut the list we’re searching in half
  • Instantiate a new BinarySearch
  • Pass the second half of the list into it
  • Call the search method on the new object, with the same search key
  • Repeat until the keys are equal
  • If they’re never equal, raise “Not Found”

With this plan in place, I started to build the method. First, I created the method and passed in the search key:

1
2
3
def search_for(key)

end

Then I checked it against the middle element in the list, returning the index if they were equal:

1
2
3
def search_for(key)
  return middle if list[middle] == key
end

The next thing on my to do list was to branch for the other two conditions: whether the key was less than or greater than the middle value:

1
2
3
4
5
6
7
8
9
10
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    # ???
  else
    # ???
  end

end

Then I filled in the branches some more of my pseudo-code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    # Cut the list we're searching in half
    # Instantiate a new BinarySearch
    # Pass the first half of the list into it
    # Call the search on the new object, with the same search key
  else
    # Cut the list we're searching in half
    # Instantiate a new BinarySearch
    # Pass the second half of the list into it
    # Call the search on the new object, with the same search key
  end

end

I filled them in with real code, one step at a time:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    # Cut the list we're searching in half
    sublist = list[0..middle]

    # Instantiate a new BinarySearch
    # Pass the first half of the list into it
    search = BinarySearch.new(sublist)

    # Call the search on the new object, with the same search key
    return search.search_for(key)
  else
    # Cut the list we're searching in half
    sublist = list[middle..-1]

    # Instantiate a new BinarySearch
    # Pass the first half of the list into it
    search = BinarySearch.new(sublist)

    # Call the search on the new object, with the same search key
    return search.search_for(key)
  end

end

Seeing some duplication here, I went ahead and refactored:

1
2
3
4
5
6
7
8
9
10
11
12
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    sublist = list[0..middle]
  else
    sublist = list[middle..-1]
  end

  return BinarySearch.new(sublist).search_for(key)

end

Time to run the tests.

1
2
Expected: 5
  Actual: 2

Fail.

Why? I did some pry action and discovered that I was getting back the index of the new list’s middle element, not the original list’s middle element. To fix this, I added the current list’s middle to the recursive call. That meant undoing my refactor. I guess I refactored too soon.

1
2
3
4
5
6
7
8
9
10
11
12
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    sublist = list[0..middle]
    return BinarySearch.new(sublist).search_for(key)
  else
    sublist = list[middle..-1]
    return BinarySearch.new(sublist).search_for(key) + middle
  end

end

The tests were now passing. Good. Time to try with another example. This time looking for keys on both sides of the middle.

1
2
3
4
def test_it_finds_position_in_a_larger_list
  binary = BinarySearch.new([1, 3, 5, 8, 13, 21, 34, 55, 89, 144])
  assert_equal 1, binary.search_for(9)
end

Whoops! stack level too deep! I forgot to change value of the search on line 3. I was getting an infinite loop when I searched for a bad value. I skipped this test and wrote one for the bad search condition, instead:

1
2
3
4
5
def test_it_raises_error_for_data_not_in_list
  assert_raises RuntimeError do
    BinarySearch.new([1, 3, 6]).search_for(2)
  end
end

To make this pass, I had to validate that the search wasn’t stuck looking through the same sublist over and over:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
def search_for(key)
  return middle if list[middle] == key

  if list[middle] > key
    sublist = list[0..middle]
    raise "Not Found" if sublist == list
    return BinarySearch.new(sublist).search_for(key)
  else
    sublist = list[middle..-1]
    raise "Not Found" if sublist == list
    return BinarySearch.new(sublist).search_for(key) + middle
  end

end

It passed! I went back and rewrote the other test:

1
2
3
4
5
def test_it_finds_position_in_a_larger_list
  binary = BinarySearch.new([1, 3, 5, 8, 13, 21, 34, 55, 89, 144])
  assert_equal 1, binary.search_for(3)
  assert_equal 7, binary.search_for(55)
end

Passed again! How about one more for good measure? I also wanted to check if the middle was still coming out right when I passed in a list with an even number of elements. All the others had been of an odd length.

1
2
3
4
5
def test_it_finds_correct_position_in_a_list_with_an_even_number_of_elements
  binary = BinarySearch.new([1, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 377])
  assert_equal 5, binary.search_for(21)
  assert_equal 6, binary.search_for(34)
end

Ok! Green!

Conclusion

Even if I didn’t know my stuff in the conference room at that moment, I’m glad that my interviewer asked me about Binary Search. It was fun to learn, and it turned out to be a little easier than I expected. I always expect algorithms to be really hard to grasp, but maybe now that I’ve done a few algorithms that use recursion, it’s not such a jump for me to understand it.

At any rate, it’s pretty cool to be able to find an element in a list with so few queries. Lately, I’ve really been enjoying pure math programming problems like this one. I’m going to try to find a few more like this to do in the near future.

Footnote

Since I had already done all of the work, I went ahead and submitted this as a new exercise for Exercism.io. So, if you use Exercism, and you enjoy doing algorithms, I’d suggest giving Binary Search a go yourself. If you get stuck, you know where to find the answer!

Refactoring 4: Remove Middle Man

Today, in my latest journey on the Refactor Tractor, I’ll be taking a look at a strategy called Move Method.

Move Method is used to help remove duplication. Sometimes, in the interest of encapsulation, it’s a good idea to make an object as unaware of another object as possible. If the first needs to access the second, though, it may have to “reach through” a third object. If this reaching is minimal, it can sometimes be a worthwhile trade off. But when it becomes an ongoing pattern, you can end up with a lot of duplication. And a lot of what my teacher Franklin Webber calls “bad touching”. The Refactoring book points out that “it’s hard to figure out what the right amount of hiding is”, but it’s easy to change your mind with refactoring. Using a combination of this strategy and Hide Delegate, you can always change it again later.

Today I’ll be practicing Remove Middle Man on another example from the Mancala app I was refactoring in my recent post: Refactoring 2 - Replace Method With Method Object.

Here are the steps to Remove Middle Man:

1
2
3
4
5
1. Create an accessor for the delegate.

2. For each client use of a delegate method, remove the method from the server and replace the call in the client to call the method on the delegate.

3. Test after each method.

That’s fairly cryptic. I think it means: just pass a reference to the distant object into the first object, and access it directly (instead of through object two).

Mancala Game View

There are many poorly-built parts to Mancala. That’s probably because I didn’t test it at all as I was writing it. At the time, I thought I was taking a shortcut. In reality, I was making a big mess that was difficult to understand and deal with. I didn’t realize it at the time, but “TDD’s primary benefit is to improve the design of our code”.

One of my poorly designed classes is called the MancalaGameView. Its job is to draw the beads in their respective pits, and print the winner to the screen, using Ruby Processing. Here’s the problem with my design: I pass in the ‘app’ object, then call through it to get information about the ‘rules’ and ‘model’ objects. Here’s what it looks like:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
class MancalaGameView

  attr_reader :app

  def initialize(app)
    @app = app
  end

  def draw_all_beads
    app.model.all_pits.each do |pit|
      draw_beads_in_pit(pit)
    end
    app.model.all_stores.each do |store|
      draw_beads_in_store(store)
    end
    print_winner if app.ruler.game_over?
  end

  def draw_beads_in_pit(pit)
    app.fill 225
    n = pit.count
    return if n == 0
    if n >= 1 && n <= 9
      draw_x_beads(pit, n)
    else
      draw_many_beads(pit)
    end
  end

  def draw_beads_in_store(store)
    fill_a_random_color(1)
    if store.id == 1
      draw_beads_in_player_one_store
    elsif store.id == 2
      draw_beads_in_player_two_store
    end
  end

  def print_winner
    app.text "Player one is the winner!", 640, 475 if app.ruler.winner == :player_1
    app.text "Player two is the winner!", 400, 250 if app.ruler.winner == :player_2
    app.text "Tie Game!", 640, 475 if app.ruler.winner == :tie
  end

end

The calls to app.fill and app.text are built into Ruby Processing. Outside of that, I’m not going to get into the details of what some of these methods, like draw_x_beads and draw_beads_in_player_one_store. Because the coupling in this app is so high, it would require pulling in too many methods.

Setting Up the Test

I’m going to stub out a few things, so that we can build a test without having to drag in the rest of the app and the Ruby Processing Gem.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
class MancalaGameView

  attr_reader :app

  def initialize(app)
    @app = app
  end

  def draw_all_beads
    app.model.all_pits.each do |pit|
      draw_beads_in_pit(pit)
    end
    app.model.all_stores.each do |store|
      draw_beads_in_store(store)
    end
    print_winner if app.ruler.game_over?
  end

  def draw_beads_in_pit(pit)
    true
  end

  def draw_beads_in_store(store)
    true
  end

  def print_winner
    return "Player one is the winner!" if app.ruler.winner == :player_1
    return "Player two is the winner!" if app.ruler.winner == :player_2
    return "Tie Game!" if app.ruler.winner == :tie
  end

end

class App
  attr_reader :model, :ruler
  def initialize(model, ruler)
    @model = model
    @ruler = ruler
  end
end

class Model
  def all_pits
    [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]
  end

  def all_stores
    [0, 1]
  end

end

class Ruler
  def winner
    winners = [:player_1, :player_2, :tie]
    @winner ||= winners[rand(3)]
  end

  def game_over?
    true
  end
end

And now, to build a test.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
class GameViewTest < Minitest::Test

  def setup
    @model = Model.new
    @ruler = Ruler.new
    @app = App.new(@model, @ruler)
    @view = MancalaGameView.new(@app)
  end

  def test_draw_all_beads_succeeds
    expected = ["Player one is the winner!", "Player two is the winner!", "Tie Game!"]
    result = @view.draw_all_beads
    assert expected.include?(result)
  end

end

The color green

This test is green. I don’t like to start from green. It’s supposed to be: “red, green, refactor”! I could comment out some code or change game_over? to false, but that wouldn’t prove anything about the code. Since this is an instance of Test After, and the code is already understood, I’m feel comfortable going forward from green here.

Applying the Strategy

Now, to begin removing the middle man object, app. There are two classes I’m accessing through app: Model and Ruler. I’ll extract them one at a time, beginning with Model. The first step is to “create an accessor for the delegate”. I could just do this:

1
2
3
4
5
class MancalaGameView
  def model
    app.model
  end
end

But I would rather pass the model into the MancalaGameView directly:

1
2
3
4
5
6
7
8
9
10
class MancalaGameView

  attr_reader :app, :model

  def initialize(app, model)
    @app = app
    @model = model
  end

end

Step two is to change “client” calls so that they use the new accessor instead of calling through the “server” (which is app in this case). Step three is to test after each method. So here we go:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
# OLD VERSION
# def draw_all_beads
#   app.model.all_pits.each do |pit|
#     draw_beads_in_pit(pit)
#   end
#   app.model.all_stores.each do |store|
#     draw_beads_in_store(store)
#   end
#   print_winner if app.ruler.game_over?
# end

def draw_all_beads
  model.all_pits.each do |pit|
    draw_beads_in_pit(pit)
  end
  model.all_stores.each do |store|
    draw_beads_in_store(store)
  end
  print_winner if app.ruler.game_over?
end

The color red

The test fails with: wrong number of arguments (1 for 2). The problem lies in the test setup. I now need to pass in the model when I create the MancalaGameView.

1
2
3
4
5
6
7
8
class GameViewTest < Minitest::Test

  def setup
    # Code omitted
    @view = MancalaGameView.new(@app, @app.model)
  end

end

The color green

Ahh, that’s better. Now to repeat the process with the ruler. Step one: create the accessor.

1
2
3
4
5
6
7
8
9
10
11
class MancalaGameView

  attr_reader :app, :model, :ruler

  def initialize(app, model, ruler)
    @app = app
    @model = model
    @ruler = ruler
  end

end

Steps two and three: replace the method calls to use the accessor instead of reaching through app. Test after each step.

1
2
3
4
def draw_all_beads
  # Code omitted
  print_winner if ruler.game_over?
end

The color red

It fails, because I’m not passing ruler into the test.

1
2
3
4
5
6
7
8
class GameViewTest < Minitest::Test

  def setup
    # Code omitted
    @view = MancalaGameView.new(@app, @app.model, @app.ruler)
  end

end

The color green

That gets me back to green. Now to refactor the print_winner method:

1
2
3
4
5
6
7
8
9
10
11
12
# OLD VERSION
# def print_winner
#   return "Player one is the winner!" if app.ruler.winner == :player_1
#   return "Player two is the winner!" if app.ruler.winner == :player_2
#   return "Tie Game!" if app.ruler.winner == :tie
# end

def print_winner
  return "Player one is the winner!" if ruler.winner == :player_1
  return "Player two is the winner!" if ruler.winner == :player_2
  return "Tie Game!" if ruler.winner == :tie
end

The color green

Success! That was pretty easy.

Further Refactoring

Now that I’ve cleaned up the MancalaGameView somewhat, I’m going to refactor a few other things. Along the way, I noticed that print_winner is pretty ugly, so I changed it to use a hash lookup instead.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
# OLD VERSION
# def print_winner
#   return "Player one is the winner!" if ruler.winner == :player_1
#   return "Player two is the winner!" if ruler.winner == :player_2
#   return "Tie Game!" if ruler.winner == :tie
# end

def print_winner
  win_messages[ruler.winner]
end

private

def win_messages
  { :player_1 => "Player one is the winner!",
    :player_2 => "Player one is the winner!",
    :tie      => "Tie Game!"}
end

I also decided to use extract method on the loops in draw_all_beads:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
# OLD VERSION
# def draw_all_beads
#   model.all_pits.each do |pit|
#     draw_beads_in_pit(pit)
#   end
#   model.all_stores.each do |store|
#     draw_beads_in_store(store)
#   end
#   print_winner if ruler.game_over?
# end

def draw_all_beads
  draw_pits
  draw_stores
  print_winner if ruler.game_over?
end

private

def draw_pits
  model.all_pits.each do |pit|
    draw_beads_in_pit(pit)
  end
end

def draw_stores
  model.all_stores.each do |store|
    draw_beads_in_store(store)
  end
end

That’s much better. Here’s the final result:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
class MancalaGameView

  attr_reader :app, :model, :ruler

  def initialize(app, model, ruler)
    @app = app
    @model = model
    @ruler = ruler
  end

  def draw_all_beads
    draw_pits
    draw_stores
    print_winner if ruler.game_over?
  end

  def draw_beads_in_pit(pit)
    true
  end

  def draw_beads_in_store(store)
    true
  end

  def print_winner
    win_messages[ruler.winner]
  end

  private

  def draw_pits
    model.all_pits.each do |pit|
      draw_beads_in_pit(pit)
    end
  end

  def draw_stores
    model.all_stores.each do |store|
      draw_beads_in_store(store)
    end
  end

  def win_messages
    { :player_1 => "Player one is the winner!",
      :player_2 => "Player one is the winner!",
      :tie      => "Tie Game!"}
  end

end

Conclusion

Remove Middle Man was really easy to use. Maybe it’s because I’ve heard Franklin harp on the evils of “bad touching” so much, or maybe it’s just that I’m violating the Law of Demeter, but I start to feel pretty nervous when I see two dots in my method calls. This was a quick and easy refactor that I’m sure I’ll use regularly. Since it sets you up for Dependency Injection, it also opens up the class’s possibilities for Duck Typing and easy mocking.

As I continue to code and refactor, Remove Middle Man will be a vital tool in my tool kit.

Refactoring 3 - Introduce Named Parameter / Introduce Class Annotation

I’ve been trying to get better at refactoring lately. To do that, I’m digging into my old code from the first months of gSchool and applying strategies from Refactoring Ruby by Jay Fields, Shane Harvie, Martin Folwer, and Kent Beck.

Just a couple of days ago I posted about trying out Replace Method with Method Object. Almost immediately afterwards, I read about two more strategies that seemed like they would fit well with the same refactoring example, so I thought I’d give them a try: Introduce Named Parameter and Introduce Class Annotation.

Introduce Named Parameter

According to the book, naming your parameters is a good thing to do if you want to improve the readability of a method. Using it prevents the work of having to go searching through dependency objects to figure out what’s going on. It works easily with the new BeadDistributor class I created in my last article. Here’s that code again:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
class BeadDistributor
  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(rules, app, first_pit_id, player, count)
    @rules = rules
    @app = app
    @first_pit_id = first_pit_id
    @player = player
    @count = count
  end

  def compute
    fix_landing_on_spot_12
    take_both_if_own_empty
    distribute_beads
  end

  def distribute_beads
    (count - 1).times do |i|
      rules.execute_store_logic(next_pit_id, player, count)
      app.add_bead_to_model(next_pit_id)
    end
  end

  def next_pit_id
    app.find_next_pit_in_model(first_pit_id)
  end

  def take_both_if_own_empty
    rules.take_both_sides(landing_spot) if landed_on_players_own_empty_pit?
  end

  def landed_on_players_own_empty_pit?
    rules.landed_on_empty?(landing_spot) && app.check_if_pit_belongs_to_player(player, landing_spot)
  end

  def landing_spot
    @landing_spot ||= next_pit_id - 1
  end

  def fix_landing_on_spot_12
    @landing_spot = 12 if landing_spot == 0
  end

end

The most important parts here will be the attr_readers and the initialize method.

Here are the steps for Introduce Named Parameter:

1
2
3
4
5
6
7
8
9
1. Choose the parameters that you want to name. If you are not naming all of the parameters, move the parameters that you want to name to the end of the parameter list.

2. Test

3. Replace the parameters in the calling code with name/value pairs.

4. Replace the parameters with a Hash object in the receiving method. Modify the receiving method to use the new Hash.

5. Test.

As always, I started with the tests. They were still passing after my last refactoring adventure:

A set of passing Ruby tests

For the purposes of this example, I wanted to make all of the parameters required, so I skipped steps 1 and 2.

Here are the relevant parts of the code again:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
class BeadDistributor
  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(rules, app, first_pit_id, player, count)
    @rules = rules
    @app = app
    @first_pit_id = first_pit_id
    @player = player
    @count = count
  end

  # Code omitted for brevity.

end

class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    BeadDistributor.new(self, app, first_pit_id, player, count).compute
  end

end

Step 3: Replace the params in the calling method with key/value pairs.

1
2
3
4
5
6
7
8
9
10
11
12
class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    BeadDistributor.new({
      :rules => self,
      :app => app,
      :first_pit_id => first_pit_id,
      player => :player,
      :count => count}).compute
  end

end

Step 4: Modify the params taken into the receiving object to take a hash.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
class BeadDistributor
  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(params)
    @rules = params[:rules]
    @app = params[:app]
    @first_pit_id = params[:first_pit_id]
    @player = params[:player]
    @count = params[:count]
  end

  # Code omitted for brevity.

end

Step 5: Test.

A set of passing Ruby tests

The test is failing, because I haven’t modified it to send a hash instead of the individual params. After fixing this, it passes again.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
class BeadDistributorTest < Minitest::Test

  def test_compute_returns_the_correct_count
    rules = MancalaKalahRules.new
    app = App.new
    id = 2
    player = nil
    count = 10
    distributor = BeadDistributor.new({
      :rules => rules,
      :app => app,
      :id => id,
      :player => player,
      :count => count})

    expected = 9
    result = distributor.compute
    assert_equal expected, result
  end

end

A set of passing Ruby tests

At this point, everything’s working. But why did I do it? If anything, it just added duplication to what was already there. Here’s why: you can combine it with the Introduce Class Annotation strategy to DRY up object initialization.

Introduce Class Annotation

Although I’ve heard from many developers more experienced than I am that metaprogramming is generally a bad idea, I also get the impression that many of them consider doing it anyway a guilty pleasure, particularly in non-production code.

However you feel about it, gird yourself, because we’re heading into a little bit of define_method territory.

I’m going to use the same annotation the book gives as an example. It’s a way of defining a helper along the lines of attr_reader, attr_writer, and attr_successor. In it, you wrap all of the object initialization code into a neat new method called hash_initializer.

Here are the steps:

1
2
3
4
5
6
7
1. Decide on the signature of your class annotation. Declare it in the appropriate class.

2. Convert the original method to a class method. Make the apppropriate changes so that the method works at the class scope.

3. Test.

4. Consider using Extract Module on the class method to make the annotation more prominent in your class definition.

Here we go.

Step 1: Declare the new class annotation.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
class BeadDistributor

  def hash_initializer(*attribute_names)
    define_method(:initialize) do
      data = args.first || {}
      attribute_names.each do |attribute_name|
        instance_variable_set "@#{attribute_name}", data[attribute_name]
      end
    end
  end

  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(params)
    @rules = params[:rules]
    @app = params[:app]
    @first_pit_id = params[:first_pit_id]
    @player = params[:player]
    @count = params[:count]
  end

  # Code omitted for brevity.


end

Step 2: Convert the method to a class method. I also called the hash_initializer at the top of the BeadDistributor class.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
class BeadDistributor
  def self.hash_initializer(*attribute_names)
    define_method(:initialize) do |*args|
      data = args.first || {}
      attribute_names.each do |attribute_name|
        instance_variable_set "@#{attribute_name}", data[attribute_name]
      end
    end
  end

  hash_initializer :rules,
                   :app,
                   :first_pit_id,
                   :player,
                   :count

  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  # Code omitted for brevity.

end

Step 3: Test.

All green!

A set of passing Ruby tests

Step 4: Extract the class annotation into a module.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
module CustomInitializers

  def hash_initializer(*attribute_names)
    define_method(:initialize) do |*args|
      data = args.first || {}
      attribute_names.each do |attribute_name|
        instance_variable_set "@#{attribute_name}", data[attribute_name]
      end
    end
  end

end

class BeadDistributor
  extend CustomInitializers

  hash_initializer :rules,
                   :app,
                   :first_pit_id,
                   :player,
                   :count

  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  # Code omitted for brevity.

end

To me, the module you get at the end is like the gift that keeps on giving. Now I have a cool utility modult that I can use in other objects that take a hash of parameters. Right away, I can think of a program where I can use it in at least six classes.

Running the tests once more, everything is still cool.

A set of passing Ruby tests

Conclusion

As I got toward the end of the fifth chapter, Refactoring got pretty deep into some method_missing and define_method metaprogramming madness. While I can’t say I’m very excited to use these things in my day-to-day refactoring, it’s nice to have them there as a reference in case I ever need them. If nothing else, tricks like the ones I tried out here help me think about the relationships between classes in new ways. Next week, I’ll try some strategies from the next chapter: “Moving Features Between Objects”.

Refactoring 2: Replace Method With Method Object

As I mentioned last week, I’m doing a series of posts on refactoring. As gSchool wraps up, I’m working on my refactoring skills, so that I’ll be a better developer when I get my first job. I’m trying some of the strategies outlined in the book Refactoring Ruby.

Today I’ll be trying out a strategy called Replace Method with Method Object. This is a good approach to take when you have a long method with many variables in it, and using Extract Method wouldn’t be practical. Essentially, it consists of removing the method from its class and turning it into a class of its own. From there, you can more easily refactor using Extract Method and other simple strategies.

Here are the steps as outlined in the book:

1
2
3
4
5
6
7
8
9
10
11
12
13
1. Create a new class, name it after the method.

2. Give the new class an attribute for the object that hosted the original method (the source object) and an attribute for each temporary vairable and each parameter in the method.

3. Give the new class a constructor that takes the source object and each parameter.

4. Give the new class a method named "compute".

5. Copy the body of the original method into "compute". Use the source object instance variable for any invocations of methods on the original subject.

6. Test.

7. Replace the old method with one that creates the new method and call "compute".

Mancala

I took a look around my Github repositories, and found this little wonder from a Mancala app I wrote in Ruby Processing during the first month of gSchool.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    next_pit_id = app.model.find_next_pit_by_id(first_pit_id)

    @i = 0
    while @i <= count do
      execute_store_logic(next_pit_id, player, count)
      break if @i == count
      app.model.add_bead_to_pit(next_pit_id)
      next_pit_id = app.model.find_next_pit_by_id(next_pit_id)
      if @i == count - 1
        landing_spot = next_pit_id - 1
        landing_spot = 12 if landing_spot == 0
        if landed_on_empty?(landing_spot) && app.model.find_all_pit_ids_on_players_side(player).include?(landing_spot)
          take_both_sides(landing_spot)
          break
        end
      end
      @i += 1
    end
  end

end

What a mess.

Setting Up Tests

To begin the refactoring process, I had to make sure the tests were in place first. This was fairly difficult, as this method is highly coupled to other methods throughout the app. For testing purposes, I added a couple of stubbed methods to KalahRules, and created dummy App and Model classes with stubbed methods of their own. Also, distribute_beads_for_player did not return anything meaningful, so I changed it to do return the value of @i.

In the end, here’s the code I used to make the test pass:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
require 'minitest/autorun'
require 'minitest/pride'
require './kalah_rules'

class KalahRulesTest < Minitest::Test

  def test_distribute_beads_for_player_returns_the_correct_count
    rules = MancalaKalahRules.new
    app = App.new
    id = 2
    player = nil
    count = 10

    expected = 9
    result = rules.distribute_beads_for_player(app, id, player, count)
    assert_equal expected, result
  end
end


class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    next_pit_id = app.model.find_next_pit_by_id(first_pit_id)

    @i = 0
    while @i <= count do
      execute_store_logic(next_pit_id, player, count)
      break if @i == count
      app.model.add_bead_to_pit(next_pit_id)
      next_pit_id = app.model.find_next_pit_by_id(next_pit_id)
      if @i == count - 1
        landing_spot = next_pit_id - 1
        landing_spot = 12 if landing_spot == 0
        if landed_on_empty?(landing_spot) && app.model.find_all_pit_ids_on_players_side(player).include?(landing_spot)
          take_both_sides(landing_spot)
          break
        end
      end
      @i += 1
    end
    @i
  end

  def execute_store_logic(pit_id, player, count)
  end

  def take_both_sides(spot)
  end

  def landed_on_empty?(spot)
    true
  end

end

class App
  def model
    Model.new
  end
end

class Model
  def find_next_pit_by_id(pit_id)
    3
  end

  def add_bead_to_pit(pit_id)
  end

  def find_all_pit_ids_on_players_side(player)
    [1, 2, 3, 4, 5, 6]
  end
end

All green! Time to start refactoring!

A set of passing Ruby tests

Applying the Strategy

Step 1: Create a new class named after the method:

1
2
class BeadDistributor
end

Step 2: Give the new class an attribute of the source object and its parameters.

1
2
3
4
5
6
class BeadDistributor

  def initialize(rules, app, first_pit_id, player, count)
  end

end

Step 3: Give the new class a constructor that takes the new attributes. I chose to add attr_readers as well:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
class BeadDistributor
  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(rules, app, first_pit_id, player, count)
    @rules = rules
    @app = app
    @first_pit_id = first_pit_id
    @player = player
    @count = count
  end

end

Step 4: Give the new class a compute method:

1
2
3
4
5
6
7
8
class BeadDistributor

  # Code omitted for brevity

  def compute
  end

end

Step 5: Copy the original method into compute. I also copied in the stubbed methods for testing purposes.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
class BeadDistributor

  # Code omitted for brevity

  def compute
    next_pit_id = app.model.find_next_pit_by_id(first_pit_id)

    @i = 0
    while @i <= count do
      execute_store_logic(next_pit_id, player, count)
      break if @i == count
      app.model.add_bead_to_pit(next_pit_id)
      next_pit_id = app.model.find_next_pit_by_id(next_pit_id)
      if @i == count - 1
        landing_spot = next_pit_id - 1
        landing_spot = 12 if landing_spot == 0
        if landed_on_empty?(landing_spot) && app.model.find_all_pit_ids_on_players_side(player).include?(landing_spot)
          take_both_sides(landing_spot)
          break
        end
      end
      @i += 1
    end
    @i
  end

  def execute_store_logic(pit_id, player, count)
  end

  def take_both_sides(spot)
  end

  def landed_on_empty?(spot)
    true
  end

end

Step 6: Test. I tested the newly created BeadDistributor class:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
class BeadDistributorTest < Minitest::Test

  def test_compute_returns_the_correct_count
    rules = MancalaKalahRules.new
    app = App.new
    id = 2
    player = nil
    count = 10
    distributor = BeadDistributor.new(rules, app, id, player, count)

    expected = 9
    result = distributor.compute
    assert_equal expected, result
  end

end

All green!

A set of passing Ruby tests

Step 7: Replace the old method by instantiating the new object, passing the old object into it, and calling compute.

1
2
3
4
5
class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    BeadDistributor.new(self, app, first_pit_id, player, count).compute
  end

At this point, I ran the test2 again. The original one passed! Yay!

Another set of passing Ruby tests

After completing the object extraction, I was free to refactor the new BeadDistributor using more common strategies. Here’s what I ended up with:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
class BeadDistributor
  attr_reader :rules,
              :app,
              :first_pit_id,
              :player,
              :count

  def initialize(rules, app, first_pit_id, player, count)
    @rules = rules
    @app = app
    @first_pit_id = first_pit_id
    @player = player
    @count = count
  end

  def compute
    fix_landing_on_spot_12
    take_both_if_own_empty
    distribute_beads
  end

  def distribute_beads
    (count - 1).times do |i|
      rules.execute_store_logic(next_pit_id, player, count)
      app.add_bead_to_model(next_pit_id)
    end
  end

  def next_pit_id
    app.find_next_pit_in_model(first_pit_id)
  end

  def take_both_if_own_empty
    rules.take_both_sides(landing_spot) if landed_on_players_own_empty_pit?
  end

  def landed_on_players_own_empty_pit?
    rules.landed_on_empty?(landing_spot) && app.check_if_pit_belongs_to_player(player, landing_spot)
  end

  def landing_spot
    @landing_spot ||= next_pit_id - 1
  end

  def fix_landing_on_spot_12
    @landing_spot = 12 if landing_spot == 0
  end

end



class MancalaKalahRules

  def distribute_beads_for_player(app, first_pit_id, player, count)
    BeadDistributor.new(self, app, first_pit_id, player, count).compute
  end

  def execute_store_logic(pit_id, player, count)
  end

  def take_both_sides(spot)
  end

  def landed_on_empty?(spot)
    true
  end

end

class App
  def model
    Model.new
  end

  def add_bead_to_model(pit_id)
    model.add_bead_to_pit(pit_id)
  end

  def find_next_pit_in_model(pit_id)
    model.find_next_pit_by_id(pit_id)
  end

  def check_if_pit_belongs_to_player(player, landing_spot)
    model.find_all_pit_ids_on_players_side(player).include?(landing_spot)
  end
end

class Model
  def find_next_pit_by_id(pit_id)
    3
  end

  def add_bead_to_pit(pit_id)
  end

  def find_all_pit_ids_on_players_side(player)
    [1, 2, 3, 4, 5, 6]
  end
end

Running the tests one more time, I saw that everything was still green. Although I could have gone on to refactor some of the other methods in the original class that distribute_beads_for_player was coupled to, I decided to call it a day.

Another set of passing Ruby tests

Conclusion

I enjoyed the Replace Method with Method Object strategy. It’s a pretty cool way to think about the microcosm/macrocosm relationship between methods and objects. Although it’s not probably useful for every refactoring case, I imagine it would come in really handy when cleaning up a hairy method like distribute_beads_for_player. I’m learning a lot from Refactoring Ruby, and I’m looking forward to seeing what other strategies it offers.

Refactoring 1: Extract Method

I’m reading Refactoring Ruby by Jay Fields, Sean Harvie, Martin Fowler and Kent Beck. As I’m heading into the last couple of months at gSchool, I want to clean up some of the code I wrote, and practice my refactoring skills. I’ll be doing a series of articles detailing the strategies I learn. This is the first.

Today I’ll be talking about the “Extract Method” strategy. Here’s how the book describes it:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

1. Create a new method, and name it after the intention of the method (name it by what it does, not by how it does it).
If the code you want to extract is very simple, such as a single message or function call, you should extract it if the name of the new method reveals the intention of the code in a better way. If you can’t come up with a more meaningful name, don’t extract the code.

2. Copy the extracted code from the source method into the new target method.

3. Scan the extracted code for references to any variables that are local in scope to the source method. These are local variables and parameters to the method.

4. See whether any temporary variables are used only within this extracted code. If so, declare them in the target method as temporary variables.

5. Look to see whether any of these local-scope variables are modified by the extracted code. If one variable is modified, see whether you can treat the extracted code as a query and assign the result to the variable con- cerned. If this is awkward, or if there is more than one such variable, you can’t extract the method as it stands. You may need to use Split Tempo- rary Variable and try again. You can eliminate temporary variables with Replace Temp with Query (see the discussion in the examples).

6. Pass into the target method as parameters local-scope variables that are read from the extracted code.

7. Replace the extracted code in the source method with a call to the target method.
If you moved any temporary variables over to the target method, look to see whether they were declared outside the extracted code. If so, you can now remove the declaration.

8. Test.

Here’s some code from a Sinatra app I wrote early on in gSchool, called IdeaBox. For this project, we couldn’t use ActiveRecord or DataMapper, but had to hand build an ORM.

I selected these methods from my Finders module. They work together to search through a YAML file for records that match a search string. The find_raw method looked really long and complex, so I thought this would be a good place to do some refactoring.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
module Finders

  def search_for(query="")
    query = query.downcase
    find_raw({'title' => query, 'description' => query, 'tags' => query})
  end

  def find_raw(query_hash)
    begin
      found = []
      query_hash.keys.each do |key|
        query_string = query_hash[key]
        database.transaction do
          result = database['ideas'].select do |idea|
            idea[key].downcase.include?(query_string)
          end
          found << result
        end
      end
      found = found.flatten.compact.collect do |raw|
        Idea.new(raw)
      end
      select_current_portfolio_only(found)
    rescue
      []
    end
  end
end

My first step was to check the tests for the method.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
class SearchingTest < Minitest::Test

  def test_it_can_search_for_ideas_with_title_description_or_tags
    IdeaStore.create({
      'title' => "Recreation",
      'description' => "Bicycles In The Park",
      'tags' => 'bike',
      })
    IdeaStore.create({
      'title' => "Sundays",
      'description' => "In the park with George",
      'tags' => 'gerschwin',
      })
    assert_equal 2, IdeaStore.search_for("park").length
    assert_kind_of Idea, IdeaStore.search_for("Gerschwin").first
  end

end

To be thorough, I added a a couple of assertions, one to test the title field search, one for cases where there’s nothing in the database, and one for a nil search.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
  def test_search_for_can_find_ideas_by_title_description_or_tags
    IdeaStore.create({
      'title' => "Recreation",
      'description' => "Bicycles In The Park",
      'tags' => 'bike',
      })
    IdeaStore.create({
      'title' => "Sundays",
      'description' => "In the park with George",
      'tags' => 'gerschwin',
      })
    assert_equal 1, IdeaStore.search_for("Recreation").length
    assert_equal 2, IdeaStore.search_for("park").length
    assert_kind_of Idea, IdeaStore.search_for("Gerschwin").first
  end

  def test_search_for_doesnt_blow_up_on_nil
    IdeaStore.create({})
    assert_equal [], IdeaStore.search_for("anything")
    assert_equal [], IdeaStore.search_for(nil)
  end

This last test broke on undefined method downcase’ for nil`, so I updated the search_for method to protect against nil, and also return empty searched don’t yield any results.

1
2
3
4
5
6
  def search_for(query)
    return [] if query.to_s == ""

    query = query.downcase
    find_raw({'title' => query, 'description' => query, 'tags' => query})
  end

With tests in place to make sure that I wouldn’t break any of the existing functionality, I began to make changes. My first step was to rename the find_raw method to find_yaml_data, reflecting its real intention. I then had to change the method call in search_for.

Beginning the extraction, I created a new method called find_in_database_for_mutliple_queries (step 1) and pulled the code from find_yaml_data into it (step 2).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
  def search_for(query)
    return [] if query.to_s == ""

    query = query.downcase
    find_yaml_data({'title' => query, 'description' => query, 'tags' => query})
  end

  def find_yaml_data(query_hash)
    begin

      found = found.flatten.compact.collect do |raw|
        Idea.new(raw)
      end
      select_current_portfolio_only(found)
    rescue
      []
    end
  end

  def find_multiple_queries_in_database
    found = []
    query_hash.keys.each do |key|
      query_string = query_hash[key]
      database.transaction do
        result = database['ideas'].select do |idea|
          idea[key].downcase.include?(query_string)
        end
        found << result
      end
    end
    found
  end

In step 3, I looked at the original method and found that the result of the code now living in find_multiple_queries_in_database was being used on the next line:

1
2
3
  found = found.flatten.compact.collect do |raw|
    Idea.new(raw)
  end

However, I didn’t see any variables used only in the new code that needed to be moved over (step 4), because I’d already moved found = []. I could therefore safely skip step 5, which helps deal with any such variables.

Turning my attention to the new method, I identified the query_hash variable as a problem, because the new method didn’t know about it. Step 6 has you turn local variables like these into parameters to the new method. Here, I decided to rename it from query_hash to queries, because the fact that it’s a hash is an implementation detail.

1
2
3
4
5
6
7
8
9
10
11
12
13
  def find_multiple_queries_in_database(queries)
    found = []
    queries.keys.each do |key|
      query_string = query_hash[key]
      database.transaction do
        result = database['ideas'].select do |idea|
          idea[key].downcase.include?(query_string)
        end
        found << result
      end
    end
    found
  end

Finally, in step 7, I made a call to the new method from the body of the old, and assigned it to the local variable needed to pass it to the next line.

1
2
3
4
5
6
7
8
9
10
11
  def find_yaml_data(query_hash)
    begin
      found = find_multiple_queries_in_database(query_hash)
      found = found.flatten.compact.collect do |raw|
        Idea.new(raw)
      end
      select_current_portfolio_only(found)
    rescue
      []
    end
  end

At this point, I went ahead and ran the tests (step 8). They were failing, but I couldn’t tell why because of the begin/end block. I decided to remove it. I wrote it when I was still pretty new to programming, and was afraid of nil.

1
2
3
4
5
6
7
  def find_yaml_data(query_hash)
    found = find_multiple_queries_in_database
    found = found.flatten.compact.collect do |raw|
      Idea.new(raw)
    end
    select_current_portfolio_only(found)
  end

Running the tests again, I could now see the error: undefined local variable or method query_hash’. I saw that I had forgotten to rename the variable in the new method from query_hash to queries`, so I fixed it.

1
2
3
4
5
6
7
8
9
10
11
12
13
  def find_multiple_queries_in_database(queries)
    found = []
    queries.keys.each do |key|
      query_string = queries[key]
      database.transaction do
        result = database['ideas'].select do |idea|
          idea[key].downcase.include?(query_string)
        end
        found << result
      end
    end
    found
  end

All of the tests now passed. Looking at my code, I now had:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
  def search_for(query="")
    return [] if query.to_s == ""

    query = query.downcase
    find_yaml_data({'title' => query, 'description' => query, 'tags' => query})
  end

  def find_yaml_data(query_hash)
    found = find_multiple_queries_in_database(query_hash)
    found = found.flatten.compact.collect do |raw|
      Idea.new(raw)
    end
    select_current_portfolio_only(found)
  end

  def find_multiple_queries_in_database(queries)
    found = []
    queries.keys.each do |key|
      query_string = queries[key]
      database.transaction do
        result = database['ideas'].select do |idea|
          idea[key].downcase.include?(query_string)
        end
        found << result
      end
    end
    found
  end

I still wasn’t happy with the complexity, so I decided to extract some more methods. I went after this bit in find_yaml_data:

1
2
3
  found = found.flatten.compact.collect do |raw|
    Idea.new(raw)
  end

I extracted it, following the same process:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
  # Step 1: Create a new method
  def build_ideas_from_raw_data
  end


  # Step 2: Copy over the code
  def build_ideas_from_raw_data
    found.flatten.compact.collect do |raw|
      Idea.new(raw)
    end
  end

  # Step 3: Look for local variables in the source method
  # Here, there's one called 'found'
  found = found.flatten.compact.collect do |raw|
    Idea.new(raw)
  end

  # Step 4: Look for temporary variables in the extracted code.
  # There aren't any.

  # Step 5: If any temps are modified, replace them with queries.
  # Again, there aren't any.

  # Step 6: Pass locals into the new method as parameters.

  def build_ideas_from_raw_data(data)
    data.flatten.compact.collect do |raw|
      build_idea_from_data(raw)
    end
  end

  # Step 7: Replace extracted code with a call to
  # the new method in the source method.
  # I also renamed the temp from 'found' to 'ideas' for clarity.

  def find_yaml_data(query_hash)
    found = find_multiple_queries_in_database(query_hash)
    ideas = build_ideas_from_raw_data(found)
    select_current_portfolio_only(ideas)
  end

Step 8 is to test. All green! Taking another look at the whole thing, I now had this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
   def search_for(query="")
     return [] if query.to_s == ""

     query = query.downcase
     find_yaml_data({'title' => query, 'description' => query, 'tags' => query})
   end

   def find_yaml_data(query_hash)
     found = find_multiple_queries_in_database(query_hash)
     ideas = build_ideas_from_raw_data(found)
     select_current_portfolio_only(ideas)
   end

   def build_ideas_from_raw_data(data)
    data.flatten.compact.collect do |raw|
      build_idea_from_data(raw)
    end
  end

   def find_multiple_queries_in_database(queries)
     found = []
     queries.keys.each do |key|
       query_string = queries[key]
       database.transaction do
         result = database['ideas'].select do |idea|
           idea[key].downcase.include?(query_string)
         end
         found << result
       end
     end
     found
   end

My next thought was to clean up find_multiple_queries_in_database. I started by replacing the #each call with a #map instead. This let me remove all references to the found local variable, and cleaned the whole thing up considerably.

1
2
3
4
5
6
7
8
9
10
  def find_multiple_queries_in_database(queries)
    queries.keys.map do |key|
      query_string = queries[key]
      database.transaction do
        result = database['ideas'].select do |idea|
          idea[key].downcase.include?(query_string)
        end
      end
    end
  end

Reflecting further, I saw that the hash I was creating in the search_for method was unneccesary, as all of the values were the same. I moved the keys out into an array, and adjusted the find_yaml_data method to accept a string rather than a hash.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
  def search_for(query="")
    return [] if query.to_s == ""

    query = query.downcase
    find_yaml_data(query)
  end

  def fields_to_search
    ["title", "description", "tags"]
  end

  def find_yaml_data(query)
    found = find_multiple_queries_in_database(query)
    ideas = build_ideas_from_raw_data(found)
    select_current_portfolio_only(ideas)
  end

Running the tests, this wasn’t working, as the find_multiple_queries_in_database method still expected a hash. I changed it as well:

1
2
3
4
5
6
7
  def find_mutliple_queries_in_database(query)
    fields_to_search.collect do |field|
      database.transaction do
        find_query_in_database(query, field)
      end
    end
  end

I didn’t like the name anymore, so I changed it to search_fields_for_query. This last refactoring was a sweeping change, and removing the hash cleaned up a lot of things. All of the tests were still green. The code now looked like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
  def search_for(query)
    return [] if query.to_s == ""

    query = query.downcase
    find_yaml_data(query)
  end

  def fields_to_search
    ["title", "description", "tags"]
  end

  def find_yaml_data(query)
    found = search_fields_for_query(query)
    ideas = build_ideas_from_raw_data(found)
    select_current_portfolio_only(ideas)
  end

  def build_ideas_from_raw_data(data)
    data.flatten.compact.collect do |raw|
      Idea.new(raw)
    end
  end

  def search_fields_for_query(query)
    fields_to_search.collect do |field|
      database.transaction do
        database['ideas'].select do |idea|
          idea[field].downcase.include?(query.downcase)
        end
      end
    end
  end

I could see that search_fields_for_query still had another candidate for extraction, so I went ahead with it, following the same steps:

1
2
3
4
5
6
7
8
9
10
11
12
13
  def search_fields_for_query(query)
    fields_to_search.collect do |field|
      database.transaction do
        find_query_in_database(query, field)
      end
    end
  end

  def find_query_in_database(query, field)
    database['ideas'].select do |idea|
      idea[field].downcase.include?(query)
    end
  end

I then realized that I could push the #downcase call from search_for fown into the database query. This cleaned up the former nicely:

1
2
3
4
5
6
7
8
9
10
11
  def search_for(query)
    return [] if query.to_s == ""
    find_yaml_data(query)
  end


  def find_query_in_database(query, field)
    database['ideas'].select do |idea|
      idea[field].downcase.include?(query.downcase)
    end
  end

Since search_for was now just delegating to find_yaml_data, I deleted the latter and pulled its code into search_for.

1
2
3
4
5
6
7
  def search_for(query)
    return [] if query.to_s == ""

    found = search_fields_for_query(query)
    ideas = build_ideas_from_raw_data(found)
    select_current_portfolio_only(ideas)
  end

As a finishing touch, I pulled the transformation of raw data into an object out of build_ideas_from_raw_data and into a new method. This increased the lines of code, but I thought it was clearer, and the new method could potentially be reused elsewhere.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
  # Before
  def build_ideas_from_raw_data(data)
   data.flatten.compact.collect do |raw|
     Idea.new(raw)
   end
  end

  # After
  def build_ideas_from_raw_data(data)
    data.flatten.collect do |raw|
      build_idea_from_data(raw)
    end
  end

  def build_idea_from_data(data)
    Idea.new(data)
  end

All tests still green. Here’s the final result:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
  def search_for(query)
    return [] if query.to_s == ""

    found = search_fields_for_query(query)
    ideas = build_ideas_from_raw_data(found)
    select_current_portfolio_only(ideas)
  end

  def search_fields_for_query(query)
    fields_to_search.collect do |field|
      database.transaction do
        find_query_in_database(query, field)
      end
    end
  end

  def fields_to_search
    ["title", "description", "tags"]
  end

  def find_query_in_database(query, field)
    database['ideas'].select do |idea|
      idea[field].downcase.include?(query.downcase)
    end
  end

  def build_ideas_from_raw_data(data)
    data.flatten.collect do |raw|
      build_idea_from_data(raw)
    end
  end

  def build_idea_from_data(data)
    Idea.new(data)
  end

Ahh, much better. Thanks, Extract Method strategy!

Looking Ahead to the Last Two Months of gSchool

We’re coming into the home stretch at gSchool. With only 56 days left, I’m reflecting on what I’ve learned so far, and trying to sort out what I want to make of the time I have left.

Goals - Where I’ve Been

My biggest goals for gSchool have been to become a great developer, and to build a solid foundation for continued learning.

I’ve worked hard to get good at pure Ruby, focusing on writing clean, DRY, test-driven code. I’ve prioritized learning how to do Rails “the right way”, and to understand its intricacies. I’ve also prioritized JavaScript as a second area of expertise, learning to use it in the DOM, in the context of client-side frameworks, and dabbled in a little bit of Node.js.

So far, I’ve been meeting all of these goals. I really enjoy working with these technologies, and am feeling more comfortable with them every day.

Goals - Where I’m Going

During January and February, I want to continue to hone my skills with Ruby and JavaScript. Now that I understand the basis of these languages, I’m working on using the best idioms and designs for each.

I’ll keep doing a lot of reading. I’ve just completed Crafting Rails 4 Applications, and I’m still reading Multitenancy With Rails, and I’m excited about starting Design Patterns in Ruby this weekend.

Pure computer science topics have started to interest me lately. I’m hoping that Ruby Design Patterns will help me understand patterns like Factories, Facades, and Observers. I’ll also continue to look at algorithms. One of my current projects is implementing a linked list in JavaScript. In the future, I’ll explore Binary Trees, the A* algorithm, and perhaps the MD5 algorithm.

Milestones

For me, the easiest way to break up the coming time is to think of it in terms of the projects we’ll complete. We’re just starting Feed Engine. After that, it’s the Service Oriented Architecture project, then the personal mastery project. Every project in gSchool thus far has built on the last. I’m looking forward to growing through each of these remaining experiences.

How I Will Succeed

There is little blocking me from succeess, in my mind. As long as I can keep my energy level up, I’ll be able to grok everything left on the docket: what it takes to build killer Rails apps, consuming APIs, feeds, and SOAs. Hopefully, I’ll pick up a few other tricks along the way. Given the good rest I got over the holidays, I feel refreshed and ready to keep working hard for the last two months, so I feel confident.

The parts of my personality that will best serve me are my stubbornness/tenacity, curiousity and resourcefulness. I find it hard to give up on problems, so I follow through on them. I’m always finding new things to learn about, and I’m good at finding ways to learn more about the things I’m interested in.

Finally, I’ve been really interested in productivity lately. Some of my strategies to “work smarter, not harder” are:

  • Avoid distractions
  • Use a todo list
  • Clarify my goals at the start of each day
  • Stay true to the pomodore timer

With the combination of my love for learning and my productivity, I know I’ll succeed in the last couple of months as a gSchool student, and will also continue to be a valuable programmer and a life-long learner.

What I’ve Been Reading

This post is a list of the books I’ve read and am currently reading.

Currently Reading

Previous Titles

JavaScript

Ruby

Design

Other

How to Find Big Directories in OS X (Darwin Linux)

My Dropbox was full this morning. Since I couldn’t afford to buy more space, I needed to clear some room. I set out to find the biggest files and delete them.

Before I got comfortable with the command line, I always downloaded some junky freeware to solve this problem. But today, I figured that there must be a smart way to do this with some arcane Linux incantation, so I did a little research and figured it out. It turned out to be really easy. Here’s what to do:

Step 1

Navigate to the folder you want to search through.

1
$ cd Dropbox

Step 2

Use the du (Disk Usage) command to get a list of the files in your current folder (indicated by a .) and their sizes. Pass the -a flag to search all the files (not just directories), and the -h flag to have it display a human readable format.

1
$ du -a -h .

Step 3

Use a pipe to send the results to the sort command. Pass -n to sort the results numerically, and -r to make the biggest files show up at the top of the list.

1
$ du -a -h . | sort -n -r

Step 4

Limit the results to only ten entries so you only have the biggest files.

1
$ du -a -h . | sort -n -r | head -n 10

Step 5

Optionally, dump the whole thing into a text file using the > shovel, so you can reference it while you are deleting files.

1
$ du -a -h . | sort -n -r | head -n 10 > file_sizes.txt

Open file_sizes.txt in your favorite text editor, and start deleting files in the terminal. To keep the text file up to date, you can hit up arrow a couple of times to get the above command back and run it again after you’ve deleted a few things.

Conclusion

Now you can find the biggest files on any part of your hard drive. Just navigate to the folder you want to look at before running the command, or replace the . with the path you want to search (e.g. $ du -a -h ~/Dropbox | sort -n -r | head -n 10 > file_sizes.txt).

Enjoy!

Contributing to Open Source & Git Squash

Open Source

I’ve been really interested in contributing to open source lately. In part, because it seems like a really good way to learn about the gems I use. Also, because I am on a campaign of giving right now in my life, and contributing is an easy way to do so in computer code (which is basically all I do/breathe/sleep right now).

I started off with something small, a contribution to the Ember.js guides. I noticed that there was an inconsistency in one step of the tutorial: sometimes they called the everyProperty() method, and other times they called everyBy(). I corrected it to use everyProperty() throughout and submitted. Even though my pull request was not accepted, because the latter method was deprecated in favor of the former, it was a magical experience. The people working on Ember were talking to me! And I was helping them make Ember better! I was feeling into it.

Over the next month or so, I submitted some other documentation pull requests. I had pull requests accepted to Sinatra, Express.js, and even Rails! After being scolded for not including “[ci skip]” in my commit message (to prevent the continuous integration build from running for a simple documentation fix), I got a tip in the form of less than 1% of a Bitcoin. This was getting exciting.

Emboldened, I decided to try for something bigger. As I was reading Addy Osmani’s Developing Backbone.js Applications, I noticed that a portion of the chapter on Marionette was not working. I dug into the source code for the book and for the Marionette example, and discovered they didn’t match. I updated the book and submitted a pull request. Several days later, it was merged. I contributed to a book! Cool!

About this time, I saw that one of my classmates’ mentors, Austen Ito, was doing the 24 Pull Requests challenge, submitting a pull request every day in December up until Christmas. I decided to jump on board.

In my pursuit of Rails Ninja status, I’d been working on learning how to use engines. Ryan Bigg’s Multitenancy with Rails and José Valim’s Crafting Rails 4 Applications got me off to a good start. An excellent start, actually. I decided to go further with my favorite source for Rails research, The Rails Guides. I was disappointed to find that the engine guide was pretty hard to read, mostly due to a lot of run-on sentences. I decided to edit the whole thing as I went.

Git Squash

Things weren’t as easy with this pull request. As I went through and made spelling and grammar fixes, I also removed some word wrapping. I submitted the pull request. The Rails contributors on Github were very nice, but were hoping I could change about seven things. I did, and submitted a new commit. Then they asked me to fix the wrapping in a separate commit, so I did that. Now I was up to four commits, when I should really only have two.

How did that git squashing thing work again?

I could have created a new branch, cherry-picked my commits, and submitted a new pull request. But I was feeling stubborn, and wanted to figure out squashing. I asked many of my friends and teachers. We tried git rebase -i, like all of the articles we found online suggested, but it wasn’t working. I’d either end up with the same commits, or lost in some kind of detached head rebasing state that I couldn’t resolve. My favorite guide to git was no help. Even git reflog wasn’t pointing the way out.

In the end, Katrina Owen came to the rescue (via email, from home). Here was my message to her:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

Hey Katrina,

Thanks for helping. I was trying to use git rebase -i for this, but not getting the results I wanted.

I'm contributing edits to the rails guide for engines. I was asked to revise my original commit, then squash it. I was also asked to send a separate commit with word wrapping.

Here's the history of the branch I'm working on:

* 0cdcf46 2013-12-11 | Word wrap to engines guide \[ci skip] (HEAD, patch-4) [Ben Lewis]
* 492ac2a 2013-12-11 | Revision to guide edits (origin/patch-4) [Ben Lewis]
* b55a803 2013-12-04 | Clarification, grammar fixes, punctuation, and capitalization \[ci skip] [Ben Lewis]
* 49ed8a1 2013-12-04 | Run Travis tests using Ruby 2.1.0-preview2 too [Santiago Pastorino]

I need to squash them so that I only have 0cdcf46 and 492ac2a (although I would like to take the message from b55a803). Here's what I want to see:

* 0cdcf46 2013-12-11 | Word wrap to engines guide \[ci skip] (HEAD, patch-4) [Ben Lewis]
* 492ac2a 2013-12-11 | Clarification, grammar fixes, punctuation, and capitalization \[ci skip] [Ben Lewis]
* 49ed8a1 2013-12-04 | Run Travis tests using Ruby 2.1.0-preview2 too [Santiago Pastorino]


What should I do?

Thanks!

Here’s what she replied:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

A separate commit, or a separate pull request?

If it's a separate pull request, here's how I'd do it:

Create a separate branch from the most recent upstream master,
and then do:

   git cherry-pick 0cdcf46

Then you can submit that branch as a separate pull request.

On your branch that you've been working on, you could get rid
of the most recent commit with

   git reset --hard HEAD~1

Then you can rebase -i with:

   git rebase -i HEAD~4 # or something

Next to the second commit that you want to squash into the first commit,
replace 'pick' with 'squash'.

Then write and quit, and it will re-open a new editor and ask you to fix
the message. It will give you both of the previous ones, and you can
pick one or the other or write a whole new message.

It was an easy fix: just use the HEAD~X for the number of commits you want to work with. Worked like a charm! I was able to squash the commits just as I wanted, and resubmit them. Next up, I had to remove the wrapping changes from the first commit (b55a803). This was a good chance to practice squashing again. This time around, I did end up using some cherry-picking magic from a separate branch, but I was still able to squash my commits. Success!

It’s crazy how hard it has been to get used to git. Probably, a lot of the overwhelm comes from all of the other things I have been learning at the same time. Anyway, now I know how to squash.

Moving Forward

I’m really interested in contributing some actual code to projects. I’ve been looking around at the issues on Rails, Sorcery, Mechanize, and other popular gems, but I often find that they’re way over my head. I was able to fix a small todo in the Exercism source code, but beyond that, I’m pretty much lost. If anyone reading this has any good leads for something that an advanced beginner can do (somewhere between documentation edits and http response parsing level insanity) please hit me up on Twitter!