Refactoring Cassowary

Now the work begins…
As I mentioned before, my aim is for this codebase to become:

clean, modern, idiomatic, tested

If Refactoring and Working Effectively with Legacy Code have taught me anything, the first step is to get tests in place so that we can “Cover and Modify” and not have to “Edit and Pray”. There are a number of ports of the Cassowary engine out there (Python, JavaScript) and I was able to get a decent amount of tests written very quickly by porting their existing test suites to C#. This left me with a lot of integration / end-to-end tests, but with relatively few unit tests – for those there was no free lunch.

Let’s get typing

After tests, the first item on the agenda was to improve the type-sanity of the code.
It made heavy use of ArrayList and Hashtable (no generics in sight) and had a lot of casting scattered throughout the code. Replacing the above with List<T>, Hashset<T>, and Dictionary<TKey,TValue> would make the code clearer, cleaner, and safer to work with. I’m a big fan of letting the compiler / type system do as much of the work as possible – it’s much less prone to idiocy than me.

foreach (var idiom …

The next change was another relatively straightforward one: be more idiomatic.
For example, in C# foreach loops are more idiomatic than for loops, and certainly more idiomatic that the direct use of IEnumerators that was to be found in the original code! As I mentioned before, this was a port from a Java implementation of Cassowary and you could tell – it felt like Java. Using bool Try...() methods rather than returning null; properties instead of Get... and Set... methods; all changes to make this codebase feel more like C# and to help future developers (including myself) fall into the pit of success.

Change is tough

And I’m not talking about the refactoring… I’m talking about mutability.
The original code had a lot of publicly mutable state, with comments warning that it shouldn’t be modified when x, y, or z – at least there were comments! Some of the very first steps to tackle this were simply making this internal/protected/private, but that was quickly followed up by removing as much inappropriate mutability as possible.

Currently I’m still wrangling with a few spots of unwanted mutability right in the heart of the solver that are turning out to be a little more stubborn than the rest.

Leave a Reply