Date archives "July 2014"

Infi Coding Dojo – Object Calisthenics

The dojo

A couple of weeks ago we held the first ever Infi coding dojo. The basic idea of a coding dojo is to get together with a bunch of coders and practice some new coding skills that you’re eager to learn, but don’t normally get around doing. Besides this being a cool goal in itself, we also thought it would just be plain fun. So we decided to organize one at our company, and after surveying some colleagues the dice was thrown to let it be about object calisthenics. We ended up with a bunch of colleagues and friends and had a great time. In this blog post I’ll dive into some of the problems we faced, and how we resolved them (or not).

The challenge was to build a simple tennis-scoring program while applying the object calisthenics rules. We picked tennis-scoring because we thought it was a fairly well understood domain (which was proved wrong pretty quickly..) and it was probably small enough to fit into a 2-3 hour session while still posing the challenges you’ll face when applying object calisthenics.  I put up a description of the challenge on github, so you can try doing it yourself.

Following the rules

Object calisthenics is about 9 or 10 rules (depending on the article you read), which are supposed to make your code more object oriented and thereby ‘better’. I first learned about it by Fred George at BuildStuff 2013 and really got me thinking about what OO is all about. In this blog, I’ll talk about our experiences with applying the rules, but if you’d like to read more theory, read the original article by Jeff Bay.

Rule 1. One level of indentation per method

This one was fairly easy to attain. Just whenever you were making a mess, your usual strategy would be to factor out the violating parts into a new method. One interesting side-effect of this was that this sometimes also removes the need of comment in front of the nesting; the name of the new method would simply take over this function, which I like.

One of the more interesting cases was whether a switch statements constitutes multiple levels of indentation. For example, one of the solutions had this code:

public static string Print(GameScore gameScore)
{
    switch (gameScore)
    {
        case GameScore.Zero:
            return "0";
        case GameScore.Fifteen:
            return "15";
        case GameScore.Thirty:
            return "30";
        case GameScore.Fourty:
            return "40";
        case GameScore.Advantage:
            return "A";
    }
    return null;
}

Now, for some reason some people felt the return statements weren’t really violating this rule, and then argued that you could remove the violation by decreasing the level of indentation of either the case or the return statement. But I guess you could do that to remove any indentation, so I, as a good sensei, ruled it a violation (this is also because Fred George forbade them in his talk, as he did with if statements..). But it raises an interesting point about the rules: sometimes it isn’t entirely clear what their goals are, making good arbitration hard. Switches then would be eliminated using either Dictionaries/Hashmaps or ifs with early returns:

public static string Print(GameScore gameScore)
{
    if(gameScore==GameScore.Zero) {
        return "0";
    }

    if(gameScore==GameScore.Fifteen) {
        return "15";
    }

    ...

    return "A";
}

or

static GameScoreMap = new Dictionary<GameScore,String> {{GameScore.Zero, "0"}, {GameScore.Fifteen, "15"} ...} //note this does not actually work, should be done from static ctor

public static string Print(GameScore gameScore)
{
    return GameScoreMap[gameScore];
}

Rule 2. Don’t use the else keyword

This one also wasn’t that hard to figure out, it was mostly resolved using the same strategies as above, that is: early returns or using a dictionary. One of the solutions applied the state pattern (which interestingly enough, introduced a violation of rule 9: don’t use setters).

Again, the question was raised what the actual goal of the rule was. Most people felt the early return strategy was kind of a hack since there is still an implicit else. On the other hand, early returns in itself have the interesting property that your mental image of the function only needs to consider 2 flows at max, which I personally find very valuable. Therefor, I think early returns are a valid strategy to apply this rule.

Rule 3. Wrap all primitives and strings

This was one of the more interesting rules. While it’s actually pretty easy to wrap all primitives in their own type, it’s kind of hard to do this AND comply to rule 9 (don’t use getters and setters). I’ll illustrate the problem based a simple clock which keeps minutes and seconds (this kind of resembles the problem found in the tennis game, but keeps the terminology simpler):

    class Clock {
        private Minutes _minutes = new Minutes(0);
        private Seconds _seconds = new Seconds(0);

        public void Tick() {...}
    }

    struct Minutes {
        int _minutes;
        public Minutes(int minutes) {
            _minutes = minutes;
        }
    }

    struct Seconds {
        int _seconds;
        public Seconds(int seconds) {
            _seconds = seconds;
        }
    }

An external source calls the tick method on the clock every second. Now, how do we implement tick? A straightforward solution would be

public void Tick() { 
    _seconds.Increment();

    if(!_seconds.Equals(new Seconds(60))) {
        return;
    }

    _seconds = new Seconds(0);
    _minutes.Increment();
}

Where Seconds.Increment and Minutes.Increment are implemented by incrementing their integer field. The problem in this piece of code is the

    if(!_seconds.Equals(new Seconds(60))) {

line, because this actually constitutes checking the state of _seconds and making a decision based on it, which is exactly what rule 9 is supposed to prevent. So, even though we’re not really using a getter here, I think it’s still sort of a violation of rule 9.

A solution in accordance with the rules would be something like this

class Clock {
    ...
    public void Tick() { 
        _seconds.Increment(_minutes);
    }
}

struct Seconds {
    ...

    public void Increment(Minutes minutes) {
        _seconds++;
        if(_seconds != 60) {
            return;
        }
        _seconds = 0;
        minutes.Increment();
    }
}

Which most of us didn’t like that much either, because we felt Seconds shouldn’t really know about Minutes. Also, this gets worse if we add hours, because then we need to add Hours to the Seconds.Increment signature as well:

class Clock {
    private Hours _hours = new Hours(0);
    private Minutes _minutes = new Minutes(0);
    private Seconds _seconds = new Seconds(0);

    public Clock() { }
    public void Tick() { 
        _seconds.Increment(_minutes, _hours);
    }
}

struct Hours {
    ...
}

struct Minutes {
    ...
    internal void Increment(Hours hours) {
        _minutes ++;
        if(_minutes != 60 ){
            return;
        }

        _minutes = 0;
        hours.Increment();
    }
}

struct Seconds {
    ...

    public void Increment(Minutes minutes, Hours hours) {
        _seconds++;
        if(_seconds != 60) {
            return;
        }
        _seconds = 0;
        minutes.Increment(hours);
    }
}

So now Seconds needs to also know about Hours, which most of us feel just isn’t right (but might be caused by our collective procedural mindset). This solution, by the way, is in violation of rule 8 (no more than two instance variables). Working around that requires you to setup an even more elaborate callback structure, which I think definitely complicates matters worse.

So, this one is really undecided. I think I’m fine with comparing primitive’s values, but I think it’s a slippery slope. I’d love to hear some discussion or examples in the comments.

BTW. In a tennis game this occurs when one of the players completes a game or a set.

Rule 4. First class collections

I don’t think this rule actually applied in any of the final solutions, but there were some WIP solutions that violated this rule and corrected it later. One of the cases where there was a TennisMatch score which had 2 dictionaries, one for the set score, and one for the game score. These where refactored in their own classes SetScore and GameScore. The corresponding methods for incrementing the applicable score for a specific player was then deferred to those objects, having GameScore call back into the TennisMatch when the game was complete. For an example, see: https://github.com/edeckers/ObjectCalisthenicsCodingDojo/blob/master/TennisMatchCodingDojo/SpelerScore.cs

Rule 5. One dot per line

I think this one pretty much followed from other rules, especially rule 9 (no getters). We found that most methods had a void return type and since there were no getters, this one was actually pretty hard to break.

Rule 6. Don’t abbreviate

We didn’t really encounter any problems with this one, except maybe for the occasional for loop iterator. But I don’t think any of those made a final solution.

Rule 7. Keep all entities small

Due to the scope of the challenge this wasn’t really a problem. Also, I suppose this one is going to be hard to break due to the other rules. If anyone has any experience with this, please let me know.

Rule 8. No classes with more than two instance variables

Again one of the more interesting rules. When applying this rule, one of the design issues that comes among what dimension you need to segregate your classes. In the case of our tennis match you can imagine having the follow class:

class TennisMatch {
    GameScore _player1GameScore;
    SetScore _player1SetScore;
    GameScore _player2GameScore;
    SetScore _player2SetScore;
}

Which violates this rules, so we need to split out some classes. Here we saw two directions, segregate by player:

class TennisMatch {
    PlayerScore _player1Score;
    PlayerScore _player2Score;
}

class PlayerScore {
    GameScore _gameScore;
    SetScore _setScore;
}

or by score:

class TennisMatch {
    ScoreInGame _scoreInGame;
    ScoreInSet _scoreInSet;
}

class ScoreInGame {
    GameScore _player1Score;
    GameScore _player2Score;
}

class ScoreInSet {
    SetScore _player1Score;
    SetScore _player2Score;
}

It turns out that segregating by score worked best since the game’s rules involve comparing scores of similar kind. For example, when deciding if a game is complete when you are on 40 in a game you need to check if the opponent’s score is not 40 or on advantage. Checking your opponent’s score is hard if it’s a couple of objects away and using object calisthenics.

I particularly liked this rule because it forces you to actually put data where it belongs. In the above situation having it on the player makes no sense except that it might feel like it’s owned by the player and should therefore be on it in. In the actual solution it of course still belongs to the player, but it’s coupled to it via identity, not reference, leading to way less coupling.

Rule 9. No getters/setters/properties

This is probably the most important rule, as well as the hardest. It forces you to really put logic where it belongs. I personally interpret this rule as: don’t make decisions on somebody else’s state, but as shown in rule 5, that becomes really hard when you involve primitives and interpret comparing (the entire object) as checking state.

Another issue that came up was whether returning a value from a (command) method counts as a getter. For example, a boolean was returned to indicate if a game was complete. Then, the calling method would increase the games for the player if that returned true. I’m a big favorite of command-query seperation myself, so usually don’t code that way. But I think this should probably count as a getter. I wonder what you guys think?

In fact, when you strictly apply Tell, don’t ask, you’ll find that there is no need for any (public) query method. You’ll always invoke operations on other objects, asking them to do stuff, and potentially call back into you. We found that this introduces some circular dependencies between classes. While it’s a common conception that this is a bad thing (a smell), there’s not really a way around this. So, also not really sure about this one.

Another issue is how to do presentation. In fact, if you look at the our example project, you’ll find there is a method returning a string representation of the score. We use this both for testing and display purposes. I think this is probably alright, but it might be seen as a violation of TDA. You could quite easily fix this changing the method to take a TextWriter and have the method write to that, though. A more interesting question that comes up is how to change formatting: what if we want to change the order in which the score are written, or want to display only the current game score. Now, this is actually a very interesting question, because if you think about it a bit more, you’ll come to the conclusion that UI and business logic will always be tightly coupled. This also implies that the business logic will need to now a thing or two about the UI. Now, this really challenges the common belief that business and UI code should and can be strictly separated. I might write more about this later, but for now I’ll refer you to an article by Allen Holub, which also addresses this issue.

Again, it would be nice if the goal of this rule would be stated somewhat more explicitly, so we can actually derive answers to these questions ourselves.

Conclusion

All in all, we had great fun doing this coding dojo. It’s always good to try new things and have discussions with other developers. With regard to object calisthenics, I think it actually pushes you to better OO design. It forces you to think about encapsulation in ways you probably didn’t before, and that will probably lead to better code. However, I strongly felt the rules as they are formulated right now are open for too much interpretation, which can also guide/force you in the wrong direction. I think it would be really helpful if there was a follow up on the original article. I must say, however, that it could also be us just interpreting the rules in the wrong way.

As said, you can find a lot of code (both the problem and the solutions) on github. Special thanks to all participants and I really hope we can have some cool discussions in the comments. I am particularly interested how other people are solving the issues demonstrated in rule 3 and 9.