Independent Study :: JUnit Analysis

For this week, I'm going to be analyzing two things: the first part of Refactoring to Patterns by Joshua Kerievsky and the code of JUnit. This is a very interesting reflection for myself, though it may not be as interesting to readers. I would highly recommend your own such reflections, though.

Refactoring to Patterns

Oh joy! Refactoring; it is sweet music to my passionate programming ears. Patterns; they are what should be put next to "elegant (n.)" in the dictionary. Merging these two things just blows my mind. This book is very dense in insight, knowledge and wisdom. I've had to read the book very slowly, very carefully in order to make sure that I'm not missing anything. It is just that good.

"I'm delighted with the result, and I think you will be too."

Martin Fowler wrote Refactoring; and Refactoring is a really, really good book. As a test of its greatness, it still stands as extremely relevant and powerful 11 years after he wrote it. However, Martin Fowler's understanding of refactoring was still in its infancy when he wrote the book; he was still learning about refactoring and how it should fit into the developer's toolkit. He knew that he had stumbled on to something really cool, really productive, really powerful. He attempted, and somewhat succeeded in laying that out in Refactoring.

He did miss the mark a little, though. He loosely defined how refactoring would fit into a craftsman's tool belt. He didn't seem to have any strong insight or knowledge about that. His book, though awesome, failed to define the Grand Unifying Theory of Everything (in Software Development).

"Now the connection between software patterns and agile development is finally told."

What Ward Cunningham was saying was: Joshua Kerievsky has presented to us a rendition of the Grand Unifying Theory of Everything (in Software Development).

You see, as in physics software development, there has been these two seemingly opposing 'theories' that I knew were a Good Thing™. First was the structure, logical Theory of Gravity Pattern Languages. These patterns seemed to suggest that design first development was the way to go. We could use patterns like lego blocks and put together loosely coupled, high cohesion architectures that just made sense. But, then, there were the other practices that were a Good Thing™ and were at odds with design first development. Test driven development, Agile management, Extreme Programming, Quantum mechanics and refactoring were all crazy, bottom up development practices that declared design first development was not the way to go (Note: When I say design first, I mean architectural design). This two were incompatible.

Along comes Josh and declares that he has the solution. Pattern languages are meant to provide a means of communication, abstraction and goals to refactor towards or to. They are not, instead, meant to encourage design first development (though, under very special circumstances, Josh argues that they are useful in that capacity). Instead of opposing the principles encouraged by Agile and Extreme Programming, pattern languages now complement them.

I realize that I'm gushing here and that my enthusiasm is a little over the top. However, the reason is that I've personally struggled with the fact that I love writing good code and I love writing good architectures. Both of these loves seemed to discourage the other--as if I had to pick one over the other. Josh has shown me a way to resolve that conflict. His way may not be the Right Way but it excites me to no end because no one else has been able to so elegantly and so simply connect those two.

JUnit

Switching gears, I want to take some time to analyze JUnit. JUnit was created by Kent Beck and Erich Gamma, both of which are leaders in the industry that I follow very closely. I expected to open up JUnit and just behold the magic that occurs within its confines. Surprisingly, I immediately started seeing some things that I did not like.

Now, let me be clear; I'm a student of programming right now. I'm still very much learning. I could be, and probably am, very wrong about the following statements. I'm not presuming to know more than Kent Beck, Erich Gamma and the contributors to JUnit. So, take all of this with that statement in mind.

All code can be found on Kent Beck's Github fork.

TestCase inheritance

The first place that I looked at was the TestCase class. It is most interesting to me because, when writing JUnit tests for Java programs, this is the class that a particular TestCase inherits. This all makes sense to me. However, the first real line of interesting code is the class declaration:

public abstract class TestCase extends Assert implements Test {

That is a dense piece of declaration but the part that interested me was extends Assert. That means that each TestCase is a subclass of Assert. The class Assert is, predictably, a class full of static assertions. So, I asked myself, "How is a TestCase a particular type of Assertion?"

First, I argued, there is the fact that on a larger scale you could look at each test case, which is itself a collection of tests, as an assertion. But, this isn't really true. A common test case pattern is to create a single test case for each class in the project: Foo is tested by FooTest. FooTest doesn't come to mind as a particular type of Assert, but rather a collection of Assertions that tests Foo.

So that didn't work. Let us do a mental exploration of what ramifications it would mean to refactor TestCase to NOT subclass Assert. That would result in TestCase having to implement the methods on Assert in order to maintain the current API and that those methods would forward the calls to Assert.assertX(). This would definitely increase the lines of code in TestCase but it would also break the Assert from the highest coupling that exists--inheritance.

Assert API

This may be being too nit picky, but I think that the argument order in the Assert methods is off. For example, consider the following method signatures

static public void assertEquals(String message, Object expected, Object actual)
static public void assertEquals(Object expected, Object actual)

The first signature, I would argue, is less than optimal. I think that in cases like this, where there is a parameter that may or may not be specified, that the parameter is the last parameter. The reason for that is quite simple: if for whatever reason I wanted to specify a message when I hadn't before, I know where to start. I don't start at the beginning, or the middle.. I start at the end. Even more specifically, in many IDEs there is an autocompletion tool. This autocomplete usually filters using type information and parameter orders. In the case of prepending, it is potentially more difficult to see what options are available to you. In the case of appending, the use of a comma will give you nearly all the options for overloading that you could need.

There is another reason, too. This one is a bit more flimsy because it is a personal preference but I think its valid. Parameter ordering is very important and there are a lot of factors that can go into it. Chief among them, though, is the importance of that particular parameter. For example, if I am writing an Assert statement, the two most important pieces of information are the expected and actual values. They are what I truly case about. However, if I'm force to stop my thought process to enter in a message string instead of focusing on what I care about, I become more error prone and/or slower.

Comments

Independent Study :: Refactoring OJMoq

I'm running about half a week behind on my posts but I hope to be able to catch up this week. For this entry, I have decided to refactor one of my own projects. I actually attempted to dig into my dark, dark repositories of code from Freshman and Sophomore year but the code that I saw horrified me so much that I decided to refactor a more recent project, instead. I may or may not visit those projects next week.

Objective-J

First, I need to give a little lip service to the language that I'll be using. It is called Objective-J and is a strict superset of Javascript. It gives javascript classical inheritance, message passing and some other nifty features like importing. Objective-J is heavily used in the Cappuccino framework and Cappuccino is the main booster for Objective-J. OJMoq is a mocking framework for Objective-J but does not rely on Cappuccino.

My Code

I quickly learned that it is much easier to refactor my own code even months after I had written it. This is because I quickly and easily realize mistakes that I made in code: I have certain habits and tendencies. I can go down a checklist in order to resolve those mistakes I commonly make. Here's that checklist (and, as I learn more, this should grow):

• Are there methods that belong in another class? (Feature Envy). If so, move those methods to the other class via Move Method. I tend to be single-minded and attempt to complete the functionality in a single location rather than thinking about the implications of the location of the code.
• Is the code DRY? If not, use Extract Method to remove duplication. I can sling code, sometimes. Usually it is when I'm "on a roll." Turns out, however, that whenever I'm on a roll I tend to do bad things: duplicate code, forget to write tests first, create feature envy (see above), etc.
• Is there code that really belongs in another class? I'm usually reluctant to break code out into a class unless I'm refactoring or starting a new feature. When I'm refactoring, I really need to investigate the responsibility of the class. If it has more than one, break the offending code out into a new class.
• Is my coupling too tight? I fall into the trap of high coupling too often when I'm slinging code (even if I'm doing TDD). The problem is that I approach code very piecewise. When I'm refactoring, I need to look at the bigger picture and the ability for the code to flex, not break.
• Am I reinventing the wheel? I do this one way too often. Sometimes when I am hacking away at code I end up doing something that is already done in another part of the codebase or libraries.
• Am I encapsulating the data correctly? I'm very guilty of this. Especially with collections. I should be encapsulating the internal representation of the data. This also goes for self encapsulation, though I'm not a True Believer in that.

Then, of course, I would go through the normal code smells run down. Is this method too long? Is this field named correctly? Does this constructor make use of the default constructor? Etc. This checklist, though, is a very targeted means of improving my code. I'm going to try to go through this checklist whenever I write a feature on any codebase from now on. I think that it'll improve my code greatly in a relatively short amount of time. Yay!

Refactoring OJMoq

So, with my checklist in hand and my project selected, I set out. The end result, I feel, is really good. I broke it up into two parts. The first part is my refactorings. The second part is additions of features to the code.

Part One

• Matching Objective-J coding standards
• Refactoring to OJMoqAssert
• Moving comparisons to OJMoqSelector
• Making OJMoqSelector control finding selectors
• Making __ojmoq_fail() a method on OJMoqAssert
• Got rid of redundant __ojmoq_findSelector
• Got rid of an unnecessary global
• Encapsulating data representation and renaming ivars
• This was to make the equals: message behave more sanely. Before a == b did not imply that b == a, which is inconsistent with the definition of equality. It made it difficult to wrap someone's head around it. So, I changed it so that a == b => b == a. Now, the numberOfArguments logic is contained in the "find:by:" selector which states that A can have arguments and still match B which has no arguments, as long as the names are equal.
• Reuse of method that I created
• Getting rid of extraneous code
• Final clean up

There are two big refactorings in this set. The first is the extracting of assertion-esque code to the OJMoqAssert class. It only has class methods but the abstraction allows for easy testing and better readability of the code. The second is the removal of feature envy from OJMoq and putting the relevant code into OJMoqSelector. It made both look better. A pivotal commit is the one with the long message. It was where I realized that having OJMoqSelectorA == OJMoqSelectorB not implying OJMoqSelectorB == OJMoqSelectorA was a bad thing.

Part Two

This part may not be interesting from a Refactoring point of view. However, as Martin Fowler points out, refactoring is often the result of wanting to add a feature to a code base. So, I wanted to see how much refactoring would be influenced by my desire to implement new features. It was definitely interesting and a more "real world" approach to refactoring. One thing that I noticed is that I had a decent set of test cases but the refactoring still introduced a few bugs that I had to fix down the road.

• Base objects now have a bigger role. Now those base objects will get passed the messages and return values from themselves.
• Updating the OJMoq API to a better, more consistent one
• Adding another test
• Fixing some errors in the deprecated methods
• Adding ability to use callbacks instead of returns

Comments

OJMoq :: v0.3 Released

Version 0.3 of OJMoq was just released. I'm going to be very brief. I'll dig into some of the more technical details in a later blog post because this is part of my refactoring independent study.

API

The API has changed. The following methods have been deprecated:

- (void)expectSelector:(SEL)aSelector times:(CPNumber)times
- (void)expectSelector:(SEL)aSelector times:(CPNumber)times arguments:(CPArray)arguments    
- (void)selector:(SEL)aSelector withArguments:(CPArray)arguments returns:(id)value

and they have been replaced with

- (void)selector:(SEL)aSelector times:(CPNumber)times
- (void)selector:(SEL)aSelector times:(CPNumber)times arguments:(CPArray)arguments    
- (void)selector:(SEL)aSelector returns:(id)value arguments:(CPArray)arguments 

While the old selectors still work, I'm not sure if v0.4 will have them. So, you should be migrating these to the new version. I changed these in order to be more consistent and predictable.

Base Objects

In v0.2, I basically rendered the base object (the object that OJMoq wraps) inert. It didn't really do anything. In v0.3, I gave developers the option to make the base object valuable. Before, moq() and moq(@"SomeString") did the same thing. That is not true now. If you pass moq() a non-existent selector, it will eat it (like before). If you pass moq(@"SomeString") a non-existent selector, it will throw an exception. Be sure to only pass objects that aren't dangerous as base objects!

Callbacks

Version 0.3 supports callbacks. Using an api that you would expect (selector:callback: and selector:callback:arguments:) it expects to be passed a single-argument function. The function will be passed an array of arguments that was passed directly as the arguments of the selector you are expecting. For example,

- (void)someAction:(id)someObject
{
    [someObject a:someA b:someB];
}

and the test

- (void)testThatXDoesSomeAction
{
    var target = [X newX];
    var myMoq = moq();
    
    [myMoq selector:@selector(a:b:) callback:function(args)
    { 
        console.log(args[0]); // a 
        console.log(args[1]); // b
    }]
    
    [target someAction:myMoq];
}

(Note: this is a bad test. It doesn't assert anything. Be sure that you're checking something!). Now we can do some cool behavioral expectations. Good. Now lets go test!

Comments

Project OSL :: Release 6

This has been, by far, our biggest release and this is likely going to be a very, very long post. Sit back and get comfortable. Go get a coffee. I know that I need one for this.

Release 6 was all about adding features. We had (and have) and very solid core of an architecture. We are very comfortable with our code base. And, we feel that we can extend almost anything on top of that. So, we aggressively chose features to implement in this release cycle. Following are those features

• Community features
  • Implement a simple messaging system
  • Implement an inbox for users to receive messages
  • Implement a broadcast system for which owners of projects can broadcast messages to users
  • Implement a commenting system for both commenting on line items and projects
• Projects should be tied to users
  • Users should not be able to localize a project that isn't theirs
  • Users should be able to create their own project based on another's
  • Users should not see other projects in their project list
  • Users should be able to search a list of projects
• The login window needed to be refactored
• The menu bar needed to be expanded. This also added the requirement for context sensitive menu items
• Resource bundles of a language needed to be selectable in some way, instead of showing resources for all languages at one time
  • Users should be able to add a language to a localization
  • Users should be able to delete a language from a localization

That list is pretty big. Surprisingly, we finished almost all of it. In order to accomplish this, the size of our code base doubled and several major refactorings were carried out. We completely ripped out the bottom end of our application and replaced it. We overhauled the controller substructure. We changed how the sidebar worked. We refactored to reduce the size of our setup (AppController). We fixed several bugs. In summary, we did a lot.

Hey, Hey Gooodbyeee

CPKeyedArchiver is a way to serialize objects in Cappuccino. It works very well and we've never found an error in the code. However, our dream when we first started to serialize objects was to be able to serialize them into JSON. The reason for this is that CouchDB, our database backend, stores data in JSON format. We couldn't get a JSONArchiver implemented in the beginning so we fell back to CPKeyedArchiver. It had served us well.

But, time to wave goodbye to our good friend CPKeyedArchiver and welcome in the latest and greatest. Chandler and I (mostly Chandler) worked for about a day and a half to get this working, but we were finally able to produce CPKeyedJSONArchiver. Gasps, awes and the look of wonder should now blanket all of your faces, right? Well, you probably don't really know how awesome it is. But it is awesome, I promise. It will take our Model layer and serialize it to JSON. We can then take that JSON, send it over the wire to our Database, and our database stores it natively. This means that we can now pick apart our Models in our database very similar to the way we could in code. It is also smaller (likely due to the fact that CouchDB stores JSON natively and handles JSON better) and will allow us to use views in our database. Then, we can get the data over the wire, unarchive it without any crazy unarchive, interpret, separate and evaluate chain. Instead, our objects come out squeaky clean in plain text format rather than an archived format.

Speaking of.. views?

This allowed us to use CouchDB as more than just a datastore. In Couch, there is a concept called views. These views are created by using a map/reduce combination on the databases inside of Couch. So, for example, I could create a listProjectNames view that would map each record to a key/value pair.

function(doc)
{
//        key          value
    emit(doc._id, {'name':doc.Name});
}    

And, now, we have a view. This view is much faster than pulling the data down for all of the projects and then getting the names for them. So now we have the power to do just about anything with our data with minimal cost. This is some really cool stuff--and I have yet to see any SQL or SQL-like syntax in this project (Now if we could only find a way to rid ourselves of the PHP).

The Community

The Community featureset was a big test for our code base. If we had made a solid, loosely coupled architecture, adding a Community shouldn't be too awful difficult. It turns out that, while we had underestimated the time it would take to create the community features, our framework didn't even blink under the pressures of all this new code.

First, we implemented an Inbox / Detail View system for the mail. We followed the architecture that came before in the Project views and quickly came to realize that we did it stupidly before. Not to be discouraged, Kyle trucked ahead and got the Inbox and Detail View working. He then subsequently implemented the messaging system whose architecture is pretty rough but stable, at the moment. We hope to revisit this in RC7 (in fact, it is already assigned to Chandler).

Users, Projects, Oh my!

In the beginning, we created the idea of a project. The "Project Metaphor," as we called it, was a serious part of the workflow process. Now that we are somewhat happy with that process, we need to make it work in the larger context. This was surprisingly easy. We just needed to attach a user to a project and then create the appropriate controller logic to remap the user interactions. I was really happy with how easily our architecture handled this. There was not much that needed to change in the model layer or view layer. The model layer just needed to be able to get users associated with a project. The view layer didn't change except for features being added (such as the project name, owner name and resource bundles in the navigation bar).

REST API: Reloaded

I mentioned previously the REST API that exists for our application. This API was pretty simple because our database was a simple datastore. But, now, we can use the full power of CouchDB because we store the data in JSON. Cool. Let us see how that works

GET    /api/project/all_docs       list of all projects
GET    /api/project/ID#########    one project
POST   /api/project/ID########     update project
PUT    /api/project                create project
DELETE /api/project/ID######       delete project

the new API looks something like

GET    /api/project/_design/views/find                 list of all projects
GET    /api/project/_design/views/find_by_name/KEY     all projects that match KEY
GET    /api/project/ID#######                          one project
POST   /api/project/ID#######                          update project
PUT    /api/project                                    create project
DELETE /api/project/ID#######                          delete project

which, for the most part, is very similar. However, how to get a list of projects is very different. Also, we can create views that are keyed by name, id, whatever. This is a very powerful API. For release 7, however, we plan to simplify things and attempt to turn /api/project/_design/views/find into /api/project/find for the API (using PHP to translate from our API to Couch's API).

Menu Bar, Login, User Experience, Oh no!

We're starting to move away from "Really Cool Concept" land to "Practical Application" land. And, when we move from the first to the second, certain things become oh-so-important. Like the CEO of Twitter, Evan Williams, recently said, "User experience is everything." OSL is starting to look more and more like a complete, functional desktop application. This is a good thing, we were wanting to get here. Now, we have to focus on the tiny details that concepts can ignore but applications need to make great. Things like the menu bar, how the application reacts to users whom are not logged in trying to do things, how many clicks or actions it takes to get to a certain location, how much overhead is in it for the user, etc.

This is really exciting for us because it is a situation that we haven't had a lot of experience in. Typical school projects hardly ever run longer than 10 weeks and 10 weeks is enough to create a "Really Cool Concept" but nothing of a "Practical Application," especially considering the skillset and time constraints on students. We've been working on this since last fall and we're moving into unfamiliar, really cool territory. We hope to get it right, but learning is a really good objective here.

Metrics

I could go on and on about this release. This post is long enough already. So I'll get on to doing the metrics.

Release LOC  Classes     Files   Methods     Tests   Methods/Class  LOC/Class    Tests/Method   LOC/Method
1       1424    27          32   69             21   2.56           52.7         0.304          20.6
2       2551    35          39   147            26   4.2            72.9         0.177          17.4
3       2578    37          40   159            38   4.3            69.7         0.239          16.2
4       3436    43          47   216            46   5.03           79.9         0.213          15.9
5       3813    53          58   220            87   4.15           71.9         0.395          17.3
6       7577    84          87   450            171  5.36           90.2         0.38           16.8

As I said before, our code went WAY up. This is due to new features, not a ballooning of our core architecture. This is expected. This is the same for the Classes, Files and Methods metrics.

We added a bunch of tests but couldn't keep up with the features. I think we have a lot of very good tests, but that there are a lot of methods that aren't worth testing.

Methods per Class went up, but I think this is just because of the adding of features. We'll need to monitor this metric to make sure it isn't jumping too high. Same thing for the LOC per Class.

Finally, you can see that our methods are getting smaller (read: better) despite all of the code we've written. We still need to test more but this is a positive sign.

Comments

Independent Study :: Refactoring Redmine, Part 3

This week (or, rather, last week) I finished Refactoring: Ruby Edition by Jay Fields and performed some more refactorings on Redmine. I discovered that the second half of the book had more value than the first and that even good code can be improved significantly.

Refactoring: Ruby Edition

After reading the first half of the book, I stated that the book didn't provide too much value beyond Fowler's original. After reading the second half, I would disagree with that assessment. The catalog continually introduced several alternative ways to do something in Ruby that you wouldn't be able to do in Java (the language of the original book).

These alternative methods are actually very powerful when put into practice. After playing with the refactorings, I believe that they give a Ruby developer better tools to refactor and could, possibly, give the Ruby developer the power to create better abstractions that allow for better code.

Some of my favorite refactorings given in the catalog include those that use the Ruby block syntax. For example, the following code

def total
    result = 0
    
    items.each do |item|
        result += item.value
    end
    
    result
end    

can be refactored into

def total
    items.inject(0) { |sum, item| sum + item.value }
end    

and I absolutely love that. As long as you know what inject means (and, while that's a bad name, there doesn't seem to be a better one), this code makes perfect sense, is shorter and very DRY. I like all three of those. In Java, to do this you would need to create a convoluted anonymous class that wouldn't necessarily improve the code.

I would recommend this book to any developer that is writing serious Ruby code, perhaps even experienced ruby developers. While experienced developers may be familiar with the Rubyisms, this book describes a methodical, structured way to introduce those Rubyisms during refactoring. For people new to Ruby, this is a must; and, for people that practice TDD, this book should be attached to your hip.

Refactoring Redmine, Part 3

This was my last encounter with Redmine and I came away happy with my work. While Redmine has some very good, solid code throughout, there were always areas that could be refactored. This particular area was a very small class file that likely was never revisited after it was written.

Step 1

Extract Methods

http://github.com/hammerdr/redmine/commit/08fb53b9ade8f021b24b1d5fa1f70627048c0970 http://github.com/hammerdr/redmine/commit/fb0cca25319efac87d3408bd45565a1eb2e674b5 http://github.com/hammerdr/redmine/commit/ef921d57bd4228313eeacb284f16978247eea95e http://github.com/hammerdr/redmine/commit/94a8bef751e41bc8e624a0f18cfd6f4d28b5635f http://github.com/hammerdr/redmine/commit/1cea1d0f50ffb65b07628198f7c38e114588630b

Step 2

Introduce Guard

http://github.com/hammerdr/redmine/commit/aebd019fa59d9abd23f6b3b964e45822b9ba2d70

Step 3

Extract Methods

http://github.com/hammerdr/redmine/commit/160d0a0392be269f7a2e043f9aea04470e09e68c http://github.com/hammerdr/redmine/commit/9c6a2c7c449695b9dcc64a0a6d2874c8ebc614a6

Step 4

Introduce Guard

http://github.com/hammerdr/redmine/commit/f5e8311724fa6e708622dfbf1a6250c5582f315a

Step 5

Rename methods

http://github.com/hammerdr/redmine/commit/51805b7d4356ad3ba150d5c3ca2240d4893f5fb1 http://github.com/hammerdr/redmine/commit/84d5285348343425910d9a67e3b8330207c8ae48

Step 6

Extract Methods

http://github.com/hammerdr/redmine/commit/e04e5d5eaeeb4020f02d0a3e7be844218eec7b3c http://github.com/hammerdr/redmine/commit/e04e5d5eaeeb4020f02d0a3e7be844218eec7b3c http://github.com/hammerdr/redmine/commit/7c4c6aa5487c406589675c7d6c64b5c6b4518771 http://github.com/hammerdr/redmine/commit/cbc0603eb08578093c6b941446d2a195ef8bd333

Step 7

Introduce Default Parameter

http://github.com/hammerdr/redmine/commit/31a6ab321640386d15c8c86b8ab6da7a0223a764

Step 8

http://github.com/hammerdr/redmine/commit/62ac466835c3f5b6820e7c52a7805a54717a294d http://github.com/hammerdr/redmine/commit/526d5fed1aa5b2f5319ee43d72d5054ff21d65e3 http://github.com/hammerdr/redmine/commit/b8ccc9807b9f5a9a6f17afc3351558566a6af275

Step 9

Rename Method

http://github.com/hammerdr/redmine/commit/6fe721d81fc0c1f1a07bde2bf56c8abb96d818ec

Step 10

Decomposing if block to guard clauses. I think that this is a great refactor for the arrow head anti-pattern. Quick and easy to get rid of.

http://github.com/hammerdr/redmine/commit/11b7b259c5039ee12a93dbad96d5f1b175efc19b

Step 11

Extract Conditional To Method

http://github.com/hammerdr/redmine/commit/563025817503459e98032596a9d4dcdad0d0963e

Step 12

Consolidating Conditional. I think that this is a really cool, effective refactor. It is obviously a mix of Extract(/Inline) Method and Consolidate(/Break Up) Conditional, but its effectiveness is more than the sum.

http://github.com/hammerdr/redmine/commit/5d755cee63561836a0e711b5d48540532daaa306

Step 13

Rename Method

http://github.com/hammerdr/redmine/commit/fe29e2c88b13125ccad69b968a38ce7c962d1550

Analysis of Redmine Refactor

I felt that this was by far my best refactor to date. The code that I refactored was not terrible. I could read the code without much struggle. However, because I applied the refactoring tool, I think that the code turned out orders of magnitude better. I could now quickly glance over the code and understand what was going on.

Comments

Cappuccino :: Using tusk for distribution

You're working with Cappuccino or Atlas, and you say something like "I really wish that Cappuccino had something like X." Then, you take the initiative and create X. It's a great idea and you execute it perfectly. It works great on your machine and now it is time to share X with the world.

How do I do that?

Before package management systems existed, the only way to distribute this was to publish your source or a binary, have people stumble on to your site and download them directly. They would follow the instructions and hopefully everything would work out. Then they would have to understand how to import the files correctly and embed them inside of their application themselves.

Along came package management systems such as rubygems and all of this became abstracted away from the user. Instead of installing into the project itself, gems could be installed in a central place on the machine. Users could simply type

gem install X

and X would be installed, usable and all while never having to know how gem works.

So I can use gems?

No, not exactly. Gems, as far as I know, works for Ruby and only Ruby. Don't get too disappointed, though. There is a very good package management system for CommonJS (CommonJS is compatible with Objective-J). It is called tusk. Currently it is an extension of the Narwhal system (if you have cappuccino, you have narwhal; if you have narwhal, you have tusk). And, just like gems you can type

tusk install X

Saweet. Except, you have to do some more work to get that working.

What do I have to do?

There are a couple of things that need to happen. First, you need to make X into a CommonJS package. The package standard was just finalized a few days ago. You can go read the standard and modify your package to meet the standard, or..

tusk init

Yep. That command creates a baseline package for you to work from. Lets look at what that creates:

X/
    /-README
    /-bin/
        -/activate
        -/activate.bash
        -/sea
    /-lib/
    /-narwhal.conf
    /-package.json

Turns out that bin and lib are not needed (unless you want an external command, not a capp library. The bin should contain a shell script to do that if you want). You can remove those. You do need to look at the contents of the package.json, though.

{
    "name": "",
    "author": "",
    "dependencies": [],
    "contributors": []
}

Name is the name of your project ("X"). Author is you. Dependencies for most Capp libraries should be ["objective-j", "cappuccino"]. And contributors are others that have hacked on your code. I also suggest adding "description" and "keywords" to the package description. Now, here is the important part. You must include "objj-frameworks" which should have the following value ["Framework"]. This is telling narwhal where your code is (we'll get to putting your code there in a second). This is OJMoq's package.json for comparison:

{
    "name": "ojmoq",
    "dependencies": ["narwhal", "objective-j"],
    "author": "Derek Hammer",
    "description": "A mocking library for Objective-J that is inspired by the Moq project.",
    "keywords": ["objective-j", "unit test", "testing", "test", "mocking"],
    "objj-frameworks": ["Framework"]
}

Where's my code?

So, we've set everything up for packaging. Except for your code. This is actually quite simple, but there are a few things you need to know. First, let's visit Cappuccino Frameworks. When you import CPObject, this is how you do it:

@import <Foundation/CPObject>

This is because Cappuccino puts the following in the "Framework" folder of its narwhal package

Framework/Foundation/CPObject.j

Do you see the simple relationship? The first part of your library is the "Framework Name" and the second part is the actual file. For most Capp libraries, it'll probably be something like this

@import <OJMoq/OJMoq.j>

So, for your library called X, you should put code in "Framework/X/." Now you're done. You will soon be about to import like this:

@import <X/X.j>

Almost? There's more?

We're almost done. Two more steps need to happen. First, you need to put your code somewhere online in a zip archive. Tusk will download from that location. If you have your code on GitHub, though, you're already set. GitHub can create zip files of you code without any additional setup.

Finally, you need to have your package pulled into narwhal. You do this by editing the sources.json and asking the narwhal guys to pull your change. There are many different ways to fill this part out depending on how you make your zipfile available, but there are plenty of examples in the sources.json and you should be able to figure it out.

Good luck and hopefully we'll see some Capp packages to play with!

Comments

Independent Study :: Refactoring Redmine, Part 2

This week I spent some time skimming through Refactoring: Ruby Edition and also put some more refactoring into Redmine. The book, unfortunately, is just a rehash of Fowler's original but it does have some nuggets of information. The continued refactoring of Redmine was both interesting and intriguing. I had a few tough decisions to make but I think that, in the end, the factoring of the code was much better than the original.

Refactoring: Ruby Edition

Jay Fields released this book back in the fall and I was really excited to grab it. It was a book specifically about one of the most interesting topics (refactoring) and in one of the most interesting languages (Ruby). I was really excited.

Unfortunately, I found that I've already read most of this book. All of the examples are, of course, in Ruby instead of Java but the examples are almost exactly the same. The catalog has, for the most part, the exact same content (I'll get to some of the differences in a second). Despite all of this, though, it is still a great reference and a great read if you haven't read Fowler's book already.

The first chapter I had already read and practiced. I did this in Chicago at 8th Light with Doug Bradbury. We went step by step through the chapter and recreated the chapter in code. It was fun.

The chapter on testing was significantly different; Jay talked about Test::Unit (and, briefly, RSpec) instead of JUnit. He gave a good overview and stuck to the spirit of Refactoring.

The catalog had a few differences that I should point out (also, I've only read half of the catalog, more next week on the other half). Specifically, Jay expands a few chapters (like Extract Method) to include several Rubyisms. This is nice because algorithms to refactor Java do not always translate completely to other languages (like Ruby).

Jay also introduced a new pattern that is very Rubyist but also very powerful.

Refactoring Redmine, Part 2

This week I'm revisiting Redmine. I really like working with this project because it

• Has a lot (and I mean a lot!) of tests. This makes my life easier.
• Has well factored code. This makes me really think about the refactorings. In situations such as the Android function, I don't need to think about the refactorings because the code is so bad that almost anything I do is an improvement.
• Is written in Ruby and Rails. I just like these two technologies.

So, once again, I set off on a random controller and attempted to refactor. I selected the Project Controller and refactored the first method I saw (ProjectController#add).

Step 1

http://github.com/hammerdr/redmine/commit/3421454a776b838ade1049194469b39585250f19

Extract Method. Nothing special about this one.

Step 2

http://github.com/hammerdr/redmine/commit/efb6bcca1cca4a326fb859cb327f7abd020f1301

Extract method and add guard. This is my really cool Ruby refactor that I love to use. Seems to improve the code a lot.

Step 3

http://github.com/hammerdr/redmine/commit/09d75cb37601ef6b1433a179f7635f402abef6dc

Renaming to make the code more clear.

Step 4

http://github.com/hammerdr/redmine/commit/cc46fc59f81a3809e23943b782371f0dc6895444

Rename Method to follow Ruby convention.

Step 5

http://github.com/hammerdr/redmine/commit/6e348b0f9ec5c2538942f2c1083629d730504db1

Move Method. The comments already stated that they wanted to do this. I agreed. I moved the method to the Project class. I was faced with a tough decision here, though. I could have NOT passed in the parent_id because the params object is accessible in the Model, as well. However, I looked at the method and accessing the params object is not a key element to the method, but is instead just a way to access a parent_id. So, I passed it in as a parameter.

Step 6

http://github.com/hammerdr/redmine/commit/02e2490bdf9bb370d48a1225838448175e6eabc5

Move Method Block. I moved this code outside of the extracted method because it wasn't really doing what the method said it was doing.

Step 7

http://github.com/hammerdr/redmine/commit/39119d1f394009290548f5293ba24d4801a0cc1d

Extract Method. Nothing really special here.

Step 8

http://github.com/hammerdr/redmine/commit/45ace4ddcea8d826212f831ca77b9fff23279dd7

Using guards instead of an if block. If you look at this and think that this wasn't positive, just wait. I end up agreeing with you.

Step 9

http://github.com/hammerdr/redmine/commit/e22895a195947c07a3991d27979de3fdb35404fa

Extracting assignment to caller. I did this so that I could try something. I end up regressing some of this. I don't think that this was really positive or negative.

Step 10

http://github.com/hammerdr/redmine/commit/2532f6c728fb4948d2d094c6a1023bc245e7d7f7

Using a guard instead of an embedded if block.

Step 11

http://github.com/hammerdr/redmine/commit/697a49392eb7de4ea2c8a2abfc570b06b1455585

Reverting my guards and if block changes. The three statements that had the if block just looked ugly. I don't like the if block (too much nesting in this method) but we're still working.

Step 12

http://github.com/hammerdr/redmine/commit/cc6cbfa7e75fcbbe85d46d3d1af1d7bc2fab6994

Reverting the pull out here. Where I wanted to go wasn't going to happen so I reverted part of that change.

Step 13

http://github.com/hammerdr/redmine/commit/7ddcdc7facaca8fd96be78c86c9334716ba2f32a

Getting rid of that nasty if/else block. I don't like the "and return" here.

Step 14

http://github.com/hammerdr/redmine/commit/0088fb804e896f037088b2a2240280d3c51ddc89

Got rid of that nasty "and return" by extract method and add guard. Didn't know exactly what to call the method, yet.

Step 15

http://github.com/hammerdr/redmine/commit/c6a690147cb97c1c66f3321b1822c95a0144ffdd

Rename method.

Step 16

http://github.com/hammerdr/redmine/commit/48ffc423b75e8a1de46dac37dd6fbc7ba3c8a9fe

Encapsulation.

Comments

Project OSL :: RC5

We released RC5 early last night. The first thing that you will notice is that this release is much more feature anemic than the rest of our releases and, amazingly, the interface looks exactly the same. This is because we worked on most of the features over the holiday break and, because of that, we wanted to take it easy. So, RC5 is about doing things we wanted to do in RC4 but didn't have time to do. This includes:

• Default actions for forms such as the feedback form and the login form.
  
• Asynchronous loading from the server so that the application doesn't lock up
  
• Testing! testing! testing!

We also did some research into some other features (such as HTML5 file uploading) that never made it into this release because of road blocks.

Asynchronous loading

While writing the code to make our loading asynchronous, there were only a handful of files that I needed to change. This was amazing to me. I've worked on projects where changing the data access layer from synchronous to asynchronous would bring the entire architecture down. However, our team has been diligent about practicing good design.

Of the handful of files that needed to be changed, none of them was view code. First, I needed to change our data access class (OLActiveRecord) and then, because the API changed I had to go and fix the controllers that were calling the old API. In no time, I loaded up the app to find that it worked flawlessly. Boy did my eyes light up!

Default actions

When Kyle said that committed default actions for all known, programmatic forms, I was glad that someone was able to figure out how and had the time to implement the sure-to-be-crazy amount of things that needed to happen. He had pulled the wool over my eyes. In most cases, it was two lines of standard code and none were more than four lines of code. Kyle has shifty eyes.. never believe a word he says!

Lessons learned

While this was a short release, I still learned some nuggets of information.

First, holiday releases are hard to get through. In part, this is due to the dispersement of the team. In part, this is due to the amazing amount of things that don't get done during the holiday breaks. Our team had planned to release on Sunday but we hadn't put together enough work to make it a viable release. So we pushed it to today and did some more work. Don't do holiday releases!

Second, holiday releases keep the ball rolling. Coming off of Christmas break has been an entirely different experience for our team than Thanksgiving break. This could be due to the fact that we are still in the middle of a term but I think a lot of it is about the fact that we were thinking about our project over break. Maybe you should do holiday releases!

Experimental changes are both fun and frustrating. When all goes well, it is extremely satisfying (asynchronous loading) but when all goes wrong, it is extremely frustrating (file uploading). While the result (nailing 2 out of 3 highly volatile, unknown scope type of features) was really good for our release, I think that throwing in 2 or 3 more stable tasks would have helped the sanity of our team as a whole. This is just a project management perspective on keeping a team together and not necessarily a reflection of the team's skill, but I think that those more stable tasks would allow for developers to stay in the groove while working on those unstable tasks in parallel.

Metrics

Metrics time! For this release, our goal wasn't to add a huge chunk of functionality, as we normally do, but to instead add a couple experimental things and write tests. So, we expected to see our tests per method to go up dramatically.

Our LOC went up from RC4, which is mostly test code LOC.

Our Classes and Files increased, which is mostly test code.

Our tests increased by nearly double, but this surprises me in the fact that while testing I thought that we had covered most of the major areas in the code. The raw numbers of tests is very low compared to the raw numbers of methods. There may be a counting error in the metrics tool (I'll talk to Chandler about it).

Methods / Class went down because of the increase in Classes from tests (this also makes me think we are counting tests as methods).

LOC/Class went down for the same reason as above.

Tests/Method went up, which was a goal, but it didn't go up enough. Again, if my hypothesis is correct, we should be calculating 92/(351/92)=0.355 instead of 92/351=0.262. That is still not great but its a more reasonable number, in my opinion. It would be really nice to have a code coverage tool to see which methods we are failing to test.

Comments

Independent Study :: Refactoring Redmine

This week I finished reading Refactoring and started to apply some more basic refactorings to Redmine. The end of Refactoring was as interesting as the beginning and Kent Beck's final chapter was really insightful. The refactorings that I applied to Redmine were more good practice at using simple steps to improve the design. Mostly, I focused on composing methods.

Refactoring, Final Chapters

The final chapters of refactoring were interesting. William Opdyke's research into refactoring and his chapter on it had several interesting pointers and thoughts on the practice of refactoring from a research perspective. One really interesting nugget of knowledge is that refactoring is a play on the mathematical term factoring.

A factoring is a way to write a mathematical equation: factoring is applied to equations in order to increase the readability of that equation in that context. For example, Newton's second law is famously known as F = ma. This is correct, but is a factoring that makes it easier to understand in the context of which it is presented (the general public). Newton actually described F = d(mv)/dt or as the rate of change of an object's linear momentum.

Kent Beck's chapter was also very good. Kent expounds the importance of refactoring as a (very powerful) tool in a developer's toolkit. However, after reading a whole book about refactoring and how it improves your code, your efficiency, the world, the universe and everything, Kent says that one of the most important aspects about being good at refactoring is knowing when to stop. Developers will often have an end goal when they refactor and that is wonderful. However, sometimes that end goal becomes too difficult, too costly, too ideal to accomplish and the developer has to make a decision: keep the refactorings or toss them. In general, the refactorings will have improved your code but sometimes the best thing is to just toss them (especially if you were regressing the code in order to make something magical happen).

As I read Beck, I wish I had the confidence to know when to stop and when to keep going. After reading that chapter and going through the refactorings to Redmine, I found myself asking "Is this too much? Or have I just barely scratched the surface?" It was difficult to judge where in the spectrum I was at any given time. Hopefully the rest of my experiences with this independent study will give me some insight into this question.

Redmine

Switching gears, I went to a Ruby on Rails project called Redmine. Redmine is an open source project management suite. I like Ruby. I really wanted to learn how to effective refactor in a dynamic language such as Ruby (hint: its not that much different, you just need *really* good tests). Redmine was a good fit because it had a significant test suite for a significant sized Rails project. What I learned was that Rails doesn't lend itself as easily to the spaghetti code monster that was the Android function, but refactoring can improve the readability of code just as much.

Step 1

http://github.com/hammerdr/redmine/commit/8ea55533023831dc456effc6ff404a47198af4dc

Extracted a Method.

Step 2

http://github.com/hammerdr/redmine/commit/85784bed9fd7009523426d2b5e7b4f2a36589f79

Extracted a Method.

Step 3

http://github.com/hammerdr/redmine/commit/6f7e8c06e96bcd78d9ff2d936ef8abafd2b0fabf

Split a Method. :find_project was doing more than "finding (a|the) project", it was finding a board associated with the project. So I split the method.

Step 4

http://github.com/hammerdr/redmine/commit/954b5a892b20c09a04fa57bd064498123dfa8d63

Extracted a Method. Moving to a different area of the code (was satisfied with the other file), I found the account_controller. There was some duplication so I extracted a method.

Step 5

http://github.com/hammerdr/redmine/commit/f0a6eb6ead70cc4a34dff770bdcadab3eaa85f82

Removed redundant code (Rubyisms).

Step 6

http://github.com/hammerdr/redmine/commit/61ce6a5867ea3182c7a590c103c05f5f7cf31da1

Then I had some trouble. So I talked myself through it and extracted a couple methods to make sure I was sane while I was doing it. My stream of consciousness thoughts are written in comments.

Step 7

http://github.com/hammerdr/redmine/commit/8c8bc970442bc22b26dc5bbb7def854a2ab377a8

Removing redundant code. else -> if can be merged to elsif

Step 8

http://github.com/hammerdr/redmine/commit/5af748768179630e2eb3f8299ddca03045cf304a

Adding guards. I could add guards, remove return statements and make the code read better. Yes, yes and yes.

Step 9

http://github.com/hammerdr/redmine/commit/b2f388aa5a54b6099547ce12e740a8e4b5e330f1

Extract method and Add Guard. This little combination was just cool. I took a long if statement, extracted it to a method with a good name, and used a guard instead of a if block. Much cleaner.

Step 10

http://github.com/hammerdr/redmine/commit/614e83eed41483df7a81380108580a3e5db85ae8

Encapsulating. Just something I forgot to do correctly the first time.

Step 11

http://github.com/hammerdr/redmine/commit/931f1717d71e976109af62edf60a50da0079c721

Add Guards. These guards are more expressive than the guards that originally existed. Cool. I like expressive.

Comments

Project OSL :: Release Cycle 4

It's been a while. Thanksgiving break and a slow start led us to have a couple of weeks without a release. But, we released a big one. This is easily our largest release in terms of changes to both the code and the users' experience.

Here are the two features that we implemented:

• Developer can upload preferred glossaries
• Localizers can view glossaries

The glossaries were the first feature that we added that existed off of the main workflow. Given our architecture and practice of refactoring, this was surprisingly easy to implement.

Refactoring

Atlas and Cibs

BeanCounter and Barista

Server administrators

Lessons Learned

Comments