Craftsmanship Tour: 8th Light

Published: ← 2010-12-15 →
Category: ← Code →
Tags: 8th Light Clojure Groupon Java Limelight ← craftsmanship ← craftsmanship tour  → ← refactoring  → ← vim  →

{.alignright .size-full .wp-image-1607 .right width=”99” height=”103”}

The second stop on my craftsmanship tour was last Friday at 8th Light. They’re a local Chicago consultancy that’s active in the software craftsmanship community, especially in building the new Chicago SC group.

I started out pairing with Colin Jones, which was a bit lucky because he’s also a vim user. After I shared the joy of matchit.vim we got down to the task at hand.

{.alignright .size-full .wp-image-1608 .right width=”176” height=”74”}

8th Light is one of a few companies that do work for Groupon, the local wunderkind. Our task was to write a small script to sync data between services for a small percentage of subscribers, but when a site has over 40 million subscribers, you can’t just hack out something fast and hope for the best.

The work needed was very similar to an existing script, but Colin’s first instinct wasn’t a quick copy and paste and tweak. We analyzed the script to find the points of commonality and extracted them to a superclass, and we grew the existing tests (yes, the one-off script already had tests) to cover the new functionality.

I get the feeling that by the time I finish this tour I’m going to be pretty familiar with every combination of testing and mocking library out there. Last Monday it was Cucumber, this time I dropped into learning rspec and its built-in mocking. I’m glad I experimented with writing my own mocking library a few years ago, I earned a deeper understanding of mocking than one library’s API and was able to explain what I was trying to do and find the right bit of rspec docs quickly.

Every Friday lunch is “8th Light University” and a developer presents on an interesting topic. Colin gave a presentation on Clojure, walking through the basics of functional programming style like map, filter, and the importance of understanding when Clojure will or won’t walk an entire sequence to produce its result (as demonstrated by using (range 10000000000000)). It ended with a little exercise to add up all the numbers between 1 and 1000 that are evenly divisible by 3 and 5, my solution wasn’t too bad, though I should’ve used reduce in place of filter/apply:

` (defn evenly_divisible? [x] (or (= 0 (rem x 3)) (= 0 (rem x 5)))) (print (apply + (filter evenly_divisible? (drop 1 (range 1000))))) `{lang=”clojure”}

{.alignright .size-full .wp-image-1609 .right width=”169” height=”44”}

That refresher of Lisp syntax and FP came in handy immediately after lunch, when I paired with Micah Martin. He was working on adding a Clojure interface to the GUI toolkit Limelight.

We refactored some Java and Clojure to remove some code duplicated logic for locating and loading GUI elements (“players”). I don’t really have any experience in Java, but it only took me about five minutes of removing duplication before I ran into hassles with the type system.

As soon I was distracted by the interesting problem of structuring code in an unfamiliar language, my hands reverted to use vim commands. We were using IntelliJ IDEA on OS X, so I flailed a bit, at one point griping “I feel like I’m wearing mittens”. It’s disorienting to lose tools I’ve tweaked and practiced with almost daily for a dozen years.

The Java started like this:

public void recruit(PropPanel panel, String playerName, CastingDirector castingDirector) { final Scene scene = panel.getRoot(); final String scenePlayersPath = scene.getResourceLoader().pathTo(“players”);

final boolean couldRecruitFromPScene = recruitFrom(panel, playerName, castingDirector, scenePlayersPath); if(!couldRecruitFromPScene) { final String productionPlayersPath = scene.getProduction().getResourceLoader().pathTo(“players”); final boolean couldRecruitFromProduction = recruitFrom(panel, playerName, castingDirector, productionPlayersPath); if(!couldRecruitFromProduction) { boolean couldRecruitDefaultPlayer = recruitFrom(panel, playerName, builtinCastingDirector, BuiltinBeacon.getBuiltinPlayersPath()); } } }

I found this code really hard to read, in part because there were nested conditionals when it’s really doing the sequential work of trying different paths. It’s a bad application of single exit point because Java is a garbage collected language. It’s OK to have multiple return points from a function because you don’t have to worry you’re leaking memory. So I rewrote it as:

public void recruit(PropPanel panel, String playerName, CastingDirector castingDirector) { final Scene scene = panel.getRoot(); final String scenePlayersPath = scene.getResourceLoader().pathTo(“players”); final String productionPlayersPath = scene.getProduction().getResourceLoader().pathTo(“players”);

if (recruitFrom(panel, playerName, castingDirector, scenePlayersPath)) return;

if (recruitFrom(panel, playerName, castingDirector, productionPlayersPath)) return;

recruitFrom(panel, playerName, builtinCastingDirector, BuiltinBeacon.getBuiltinPlayersPath()); }

There are fewer variables and I moved their initialization together, so your eye picks up the parallel structure and just focuses on the differences.

Of course, that’s not just parallel structure but repeated code in the conditionals and call to recruitFrom(). I fell into Ruby syntax trying to explain what I wanted to do as the next step of refactoring:

Now the code doesn’t have any extra variables or repeated method calls. If the recruitFrom signature changed, there’d be exactly one place to change it.

Alas, it was not to be. I couldn’t simply bundle a player and a canvas in an array of tuples because they were of different types. Micah started typing a simple inner class to gather them together, but I vetoed it. Even though it would removtge duplication, I didn’t think it worth the tradeoff of added complexity of a new, data-only class. We left it at the previous revision.

Refactoring isn’t a blind set of tools to apply while (code.has_refactoring_possible?) because they’re about managing complexity by moving it to explicit places without duplication. You can spend hours smoothing things out only to be left with a stubborn bit of complexity that you can leave a warning on rather than spend a disproportionate amount of time trying to get that last bit. I find that when I do this, a future change will often expose what the real trouble is and I can advance the refactoring then.

All in all, a great day at 8th Light. I’m still looking for places to visit in Chicago before the end of the year, so please check out my calendar and suggest places here or on Twitter.