The Turnstone's Bill

Code Confession : Multi-purpose Methods

I recently read the book Clean Code by Robert Martin. I really enjoyed it, especially the parts where he shows you some dirty code and tears it apart, refactoring and refactoring until it’s a thing of beauty. While reading these sections I couldn’t help but notice that many of the bad code practices were things I’m guilty of.

I’m going to try confessing my own coding sins in the hope that by unpicking bad code I’ll learn more about exactly why it’s bad.

So here’s my first confession.

The bad code

Here’s a category method on NSManagedObject that I wrote.

1
2
3
4
5
6
7
8
9
10
- (NSManagedObjectID*) permanentObjectID {
  NSManagedObjectID *theID = [self objectID];
    if ( [theID isTemporaryID] ){
        NSError *err=nil;
        BOOL success = [[self managedObjectContext] obtainPermanentIDsForObjects:@[self] error:&err];
        if (!success)
          // Deal with error ... that's another story
    }
    return [self objectID];
}

Looking at this now is rather painful. Why the heck did I write it in the first place?

Well here’s my excuse. I use permanent NSManagedObjectID’s to lookup specific objects without holding on to the actual NSManagedObject. I also use them to save object identity information in preferences (eg the last viewed object etc). The problem is that the standard cocoa method objectID is not guaranteed to return the same NSManagedObjectID unless that object id has first been made permanent. Since I rely on object id’s being permanent in quite a few parts of my code, I ended up repeating the block of code above in many places. In the interest of keeping to the DRY principle I decided to capture it all in a method.

I can see three main things wrong with this code;

  1. The name permanentObjectID suggests simple property access but this isn’t what the method does
  2. It’s a function that does two very different things.
  3. The method doesn’t always return a permanent object id (eg if an error occurs) but it’s name suggests it should

These problems collaborate to produce WTF code at the call site because sometimes I’ll be calling permanentObjectID with the expectation that the id is already permanent. At other times I might know full well that the id is not permanent but will call this method to make it so. This greatly reduces readability and searchability of code because the method name no longer describes the purpose of the method call. One reason that this is important is because making the object id permanent at the wrong point can lead to a mismatch between objectID’s of the same object on different NSManagedObjectContexts.

My solution was to make two separate methods like this;

1
2
3
4
5
6
7
8
- (NSManagedObjectID*) permanentObjectID {
    NSManagedObjectID *theID = [self objectID];

    if ( [theID isTemporaryID] )
        return nil;

    return theID;
}
1
2
3
4
5
6
7
8
9
10
11
- (void) makeObjectIDPermanent {
    if ( [[self objectID] isTemporaryID] ){

        NSError *err;
        NSManagedObjectContext *moc = [self managedObjectContext];

        BOOL success = [moc obtainPermanentIDsForObjects:@[self] error:err];
        if ( !success )
          // Deal with error
    }
}

Although this leads to slightly more code, it means that these two methods don’t overlap at all. It also allows me to be really explicit when making an object id permanent and to make the permanentObjectID method behave in a way that befits its status as a noun (ie it should behave like a property).

Summary

I think this case is a pretty good example of a Schizophrenic method. It had two really different purposes, and one of them was pretty much completely hidden by the method name. The mistake was reasonably easy to see in this example, but I’ll be on the lookout for more subtle examples of this kind of thing in my code from now on.

Tricks With Rsync Filter Rules

The rsync program is one of the most powerful, but complex unix tools. It’s man page describes well over 100 commandline options as well as a fully fledged file filter syntax. Basic usage of the program isn’t too complex, but by taking the time to understand some of rsync’s less known options, and especially it’s file filtering syntax it’s possible to achieve almost complete control over what files are synced and how. In this post I’m going to outline a couple of examples of neat things that can be achieved with rsync filter rules. I won’t be going over the basics of filter rule syntax though because that’s actually really well described in the rsync man page itself.

Include only files matching a specific pattern

Given that this seems like a common use case you’d think it would be easy, but it turns out that some rsync filtering magic is needed to make this happen. Remembering that the order of filter arguments is important we could include only .html and .png files in a recursive transfer with the following additional filters (in order from first to last).

--include=*.html  
--include=*.png
--filter=-! */  

The first two include options are pretty self explanatory, and are needed to explicitly include the files we want. By default however all the other files (ones we don’t want) will be included anyway so the last option is designed to explicitly exclude those. The last option does more than just exclude though, let’s unpick it. The - at the start of the rule specifies that it is an exclude rule. The ! means only apply the exclude rule for files that don’t match the pattern, and the pattern */ matches all directories. In other words this last rule will exclude any file that is not a directory. We need to avoid excluding the directories because if those aren’t included rsync won’t even scan them to look for file we actually want synced (eg the .html and .png files).

And finally to keep things clean and avoid copying unnecessary directories we prune empty directories from the transfer by adding this option.

--prune-empty-dirs 

Better excludes with the “perishable” modifier

Recent versions of rsync allow filter rules to be flagged with a perishable modifier. The rsync man page describes this modifier as follows;

A p indicates that a rule is perishable, meaning that it is ignored in directories that are being deleted

This is particularly useful in cases where you would like to ignore the presence of a file unless that file happens to be the last remaining file in a directory (and would otherwise block deletion of a directory). A great example of this are .DS_Store files which are only used to tell the OSX Finder about how to display directories. Usually when rsyncing between folders on two different macs you would want to ignore .DS_Store files, perhaps with an exclude filter since these files are only useful on the computer they were generated on. This becomes a problem though when paired with the --delete option because .DS_Store files will block deletion of otherwise empty directories and generate unpleasant rsync warnings from rsync like Unable to delete non-empty directory.

With the perishable modifier this issue can be solved in an elegant way by adding a filter rule for .DS_Store files like this;

--filter='-p .DS_Store'

which tells rsync to exclude .DS_Store files except when the .DS_Store file is the last file in a directory.

Beware of hide rules and “delete”

This one is just a cautionary note because failure to understand the difference between rsync’s hide and exclude rules could lead to unwanted deletion of files. OK, so what is the difference between hide and exclude? From rsync’s point of view an excluded part of the file tree is something it should not examine or touch in any way, whereas a hidden part of a file tree behaves as if those files were actually not present. These might sound like the same thing, but when deletion comes into play they are quite different. For example the rsync command;

rsync -r --delete-after --filter='H /some/dir' left right/

will delete all files under right/some/dir if any exist regardless of whether equivalent files exist in left/some/dir. Another way to understand hide filters is to note that a combination of hide and protect (see below) is equivalent to exclude. So for example the following are equivalent

#Exclude rule
--exclude='/some/dir'

#Combined Hide and protect
--filter='H /some/dir' --filter='P /some/dir'

Testing Cocoa Apps With Ruby and Applescript

In my last post I wrote about setting up a jenkins server and said that one of my major motivations for doing so would be to automate my whole application tests. In this post I’m going to describe how to write those tests using Ruby’s rspec testing framework and why I chose to do so.

From Applescript to Ruby

For quite a while I’ve used a set of whole-app tests based on applescript and written using the ASUnit testing framework. This approach is pretty appealing since Applescript is the standard way to script Cocoa apps and ensures that tests are based on the same scripting mechanism as is provided to users. Over time though I found that I simply wasn’t writing new tests, and a quick look at the tests I had would reveal that they hardly covered any of my App’s functionality. The main reasons for this (apart from my laziness) are that;

  • Applescript feels verbose and its syntax is unfamiliar to most programmers
  • ASUnit is a pretty bare-bones testing framework
  • My tests were limited to the functionality I was willing to expose to scripters

The bottom line is that I simply wasn’t effectively testing my app. I felt like I needed a fresh approach, and chose a system based on Ruby and it’s popular rspec framework. After translating all my old tests to this new system I found that they were much more succinct and readable. I was also able to quickly add new tests and have finally started to see the benefits of better test coverage through early identification of bugs.

More readable tests with rspec

My goal was to make writing and reading my tests much more enjoyable. I was already familiar with the rspec framework for ruby and since I find ruby code enjoyable to write and read, I decided to start by rewriting all my Applescript tests using rspec (testing Cocoa apps via rspec isn’t limited to Applescript functionality though). My goal was to be able to write tests something 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
include 'spec_helpers.rb'
describe "my app" do
  include_context :clean_startup
  include_context :clean_shutdown

  describe "object" do
      before :each do
          @the_object = applescript_create_the_object @app_context
      end

      describe "some action on the object" do
          before :each do
              # (eg set properties, call method etc)
          end

          subject {@the_object}
          its(:property1) { should == "expected value"}
          its(:property2) { should == "expected value"}
          #...
          #...
      end

      # Testing other aspects of the object

  end
  # Testing other objects
end

There are lots of nice things about rspec, (and probably lots more I don’t know about). The example above makes use of nested describe blocks reflecting the object hierachy I want to test App->Object->Property.

Another nice thing about rspec is that it provides lots of mechanisms for organising the code required to prepare objects for testing. This preparation phase is often fairly involved and in some cases it involves expensive operations such as launching the app or performing some file operations. Being able to use nested contexts in rspec means that I can organise my preparatory code in a hierarchy of expense. For example I want to relaunch the app with a clean database between each high level group of tests, but I don’t want to do this to test each property of an object (simply creating a fresh object would do).

Finally rspec (and ruby) provide lots of mechanisms for making tests read nicely. In particular it’s easy to write custom rspec matchers, so for example I can use the following highly readable syntax to test if files exist where I expect them to be (useful for DropSync syncing tests).

1
2
3
4
5
6
7
8
"/a/path".should exist_as_path

# exist_as_path is a custom matcher written as follows
RSpec::Matchers.define :exist_as_path do
  match do |path|
          Pathname.new(path).exist?
      end
end

Ruby and Cocoa

There are two major projects that allow you to send messages to Cocoa objects using Ruby, RubyCocoa and MacRuby. RubyCocoa is the older of the two projects and works by creating proxy objects in Ruby that forward messages to Objective-C instances. MacRuby is quite different. It is a complete implementation of the Ruby language on top of the Objective-C runtime, which means that when you instantate an object from one of the Cocoa frameworks in MacRuby you’ll actually be working with the object itself and not a proxy. It also means that MacRuby comes with its own ruby interpreter macruby, and its own rspec.

Since MacRuby is eventually supposed to replace RubyCocoa and seems to be under very active development I initially figured it would be the best choice (I found this post by Steve Madsen particularly helpful when getting started). While I was initially pretty happy with this approach I ran into some nasty runtime crashes when I tried to use some xml parsing gems, and when I tried to send messages over Distributed Objects (for JSTalk). I’m sure that a seasoned macruby user could work around these issues, but I found that I was easily able to switch all my tests to RubyCocoa so I did. I’ll most likely switch back to MacRuby one day as my test code is mostly independent of the approach chosen.

Talking Applescript

Apple provides two classes that allow you to send Applescript to Cocoa applications, SBApplication and NSApplescript. While SBApplication lets you access all the methods in your application’s scripting dictionary, you don’t actually use Applescript syntax to do so. NSApplescript on the other hand is able to take a string of applescript code and run it as though it were a standalone applescript. I use both classes in my tests. SBApplication is particularly useful for inspecting object properties, whereas NSApplescript is great if you actually want to test drive your app just as an Applescript user would.

I use SBApplication to launch and access my application like this

1
2
built_dropsync_url=NSURL.fileURLWithPath(built_dropsync_path)       
dropsync = SBApplication.applicationWithURL(built_dropsync_url)

Then I manipulate my app with some actual Applescript code using NSApplescript like this

1
2
3
4
5
6
7
8
9
10
11
12
13
# Returns result on success and a dictionary of errorInfo on failure
#
def send_applescript code
  as=NSAppleScript.new.initWithSource(code)
  result, errorInfo=as.executeAndReturnError_
  if ( result==nil)
      return errorInfo
  else
      return result
  end
end

send_applescript "tell application \"DropSync\" to make new store"

And finally I use the application instance I created with SBApplication to check that the store object was actually created

1
dropsync.stores.length.should eql 1

JSTalk.ing to the rest of the app

Creating an Applescript dictionary that allows users to comprehensively drive your app is alot of work. For my app I’ve created a scripting dictionary that tries to be reasonably comprehensive but is still limited to what users might realistically want scriptable. For testing purposes though, I want much more control, and that’s where JSTalk steps in. JSTalk is a project by Gus Mueller that vends a root object in your app and allows that object to be messaged using Distributed Objects. While JSTalk is primarily aimed at scripting apps with Javascript it’s easy to use ruby instead. I’m currently only using JSTalk in a few tests to fill gaps in my Applescript functionality but I expect to be using it more as my tests become more comprehensive. If you’re wondering how to use JSTalk with ruby just download the JSTalk project and checkout Gus’s examples. He’s got a handy JSTalk.py script in there that can easily be translated to Ruby.