Inconvenient Accessibility Makes Self-Documenting Code

Intentional use of access modifiers (public, private, etc.) is like a clear memo to your team. This came up during Steve Bohlen's Virtual Alt.Net talk on domain-driven design.

Steve explained the distinction between Entity objects, which have a unique identity independent of their properties (Even when I change my name, I'm still me.), and Value objects, which are defined by their properties (If you change the house number in an address, you have a new address.). When dealing with Entities, code should not be able to change the unique id—that would be like someone claiming your social security number and thereby becoming you. Therefore, Entity classes should have private setters for their unique identifiers.

A meeting attendee asked why, since this gets inconvenient when you're creating an object based on a record fetched from the persistence repository. It's a big pain; why bother? The analogy I would offer is this. When you're defining a class to represent an Entity in your business domain, you know it's an Entity. You intend for it to behave and be treated like an Entity. You don't want any of your teammates setting its unique id in their code. So you send them an email: "Don't set Person.UniqueId, okay?" Uh hunh. How well is that going to work over time?

Instead, if you simply don't provide a public accessor to the UniqueId property, your teammates will get the message loud and clear. Granted, someone could edit the code and change the accessibility, but the fact that he or she needs to is a flashing neon sign saying "Stop. Think. Are you barking up the wrong tree?" You've made your code communicative. Its structure conveys your intent. No need for comments; this is an example of self-documenting code.

Giving Mono to my Husband

Holy crossed-platforms, Batman! How did I not know about Mono, the free, open-source framework that will run .NET applications on Linux and Mac OS X?

Not to get too personal, but I'm part of a mixed marriage: I run Windows and develop primarily in C#; my husband runs OS X and is not (actively) a programmer. Through love and mutual respect, we make it work. But what we have so far not been able to make work is my writing handy utilities and toys that he can use on his laptop.

I learned about Mono in Rod Paddock's intro to the May/June issue of CoDe Magazine. Then I came home, had Jon install Mono on his Mac, and gave him a quick little console app I'd written in C#. It ran like a charm. A WinForms app with one button and a popup message also ran, looking distinctly X11-y.

This is super exciting for us. We've been talking about a card-game playtest simulator, to help with his creation of card games. (Jon posts one free board game a month and has a few upcoming commercial releases.) That process usually involves a significant investment in card stock and time with the paper cutter, just to see how hands of cards come together and move through the game. A simulator would help him to vet the first and maybe second drafts of the cards without printing them out. Now that I know I can build something he'll be easily able to run, it's time to start designing!

The Null Object Pattern: When a slacker is just what you need

I had a challenge that was neatly solved by the Null Object pattern. I'd like to share it with you, so that I can explore the idea and provide a practical example.

Simplifying a bit, I have a Person object, and I need to fill it with details retrieved from an external system. When I started looking at the code, the call to look up the Person attributes took a Person as a passed-in parameter and modified it. That struck me as bad behavior ("Hey! I gave you that so you could use it, but I didn't expect you to change it. Sheesh."). I thought it would be more honest for the method to return information, which the controlling class could use to update the Person object if it chose to.

Let me name the players, to make this easier to follow. I have a class coordinating activities that, in our business context, is called a Translator. I created a Client that makes the actual calls to the external system. Before the refactoring, the Translator would call Client.Lookup(Person). The Client would create a message to the external system, get back a response, and use it to set attributes in the Person.

I changed Client.Lookup so that it does not change the Person, and instead returns a Response that contains the needed attributes. But if the external system did not have any info to return, should Lookup return null, throw an exception, ...?

Usually the most appropriate answer to this question is to throw an exception. If no info means you're in an invalid state or an unknown state, then it is not safe to continue, and the code should throw. In this case, though, we could continue. We didn't require the info coming back from the external system; it was just handy if available.

So I return null? But that means every time I call Client.Lookup, I have to check whether the Response is null before I use it. And so does anyone else who might be calling it in the future. It seems disingenuous for a method to say, "I'll give you a Response, but I might actually give you an empty bag. I hope you've guessed I might do that and planned accordingly."

   12 public void DoFancyBusinessSteps(Person person)

   13 {

   14     Response response = client.Lookup(person);

   15     if (response != null)

   16     {

   17         person.Address = response.Address;

   18         person.ExternalSystemId = response.ExternalSystemId;

   19     }

   20     //More stuff based on the Person...

   21 }

I'd rather return an object that is safe to use, regardless of what answer we got from the external system, and is helpful if we received useful info. This is the Null Object pattern.

I created an IResponse interface that exposes one method, Update(Person). Next I created two implementations of that interface, a Response and a NoDataResponse.

   12 public void DoFancyBusinessSteps(Person person)

   13 {

   14     IResponse response = client.Lookup(person);

   15     response.Update(person);

   16 

   17     //More stuff based on the Person...

   18 }

Response.Update uses its fields to set properties on the Person (with a method name that clearly states it is doing so). NoDataResponse.Update quietly does nothing. This allows the Translator to ask the Client to look up info about the Person, and ask the resulting Response to update the Person.

   21 public class NoDataResponse : IResponse

   22 {

   23     public void Update(Person person)

   24     {

   25     }

   26 }

I like it. As with all good tools, it's prudent not to over-use it. If quietly doing nothing would leave the Person object in a bad state, so that it blew up or corrupted data when you tried to use it later, then don't use the Null Object pattern. Throw an exception instead. The Null Object pattern is handy when you want to return an object that can do stuff in some conditions and will be harmless in other conditions.

Pretty code: Skeptical of elses

Pretty code is readable code. One strategy for code beautification is to look critically at every if/else statement. Is there a more streamlined way to write that statement? Cultivate a general mistrust of elses.

Some examples...

Testing a boolean to return a boolean

if (x == true)
{ 
  return true;
}
else 
{
  return false;
}

becomes

return x;

If you need to swap those,

return !x;

That's a pattern. Recall that comparison operators (less than, greater than, equal to) return a boolean. So this also applies here:

if (myPropertyToCheck == someValue) 
{
  return true;
}
else
{
  return false;
}

becomes

return myPropertyToCheck == someValue;

Guard clauses
You can use a guard clause when you are testing if it is safe to do something, and if not, you want to exit.

if (safeToDoThis)
{
  DoThis();
}
else
{
  return;
}

becomes

if (!safeToDoThis) return;
DoThis();

When the DoThis() is rather involved, guard clauses greatly help the readability for your future teammates (even when that's you). Step 1, check if we're in an okay state, and if not, just get out of there. This saves you from the Hunt The Else game (although, if that matching else is that hard to find, you could break up your method into smaller pieces with more specific responsibilities).

Comparisons
Compare() and CompareTo() methods need to return -1, 1, or 0. It's handy that these are integers, because now you can harness the Power of Arithmetic to do your bidding. Also when you are comparing your own classes, you often want to compare a property that is a value type or a string. Those already have their own CompareTo() methods, which you can borrow.

Say you want to sort your children by their ages. You don't need

if (child1.age < child2.age)
{
  return -1;
}
else if... 

(Not just an if/else, but an if/elseif/else. Aaah!) Use instead:

return child1.Age.CompareTo(child2.Age);

If you want to sort them from oldest to youngest, this is where the arithmetic comes in. To flip a negative 1 into a 1, multiply it by negative 1. Same for flipping positive 1 into a negative 1. And zero, conveniently, doesn't mind being multiplied by anything. So you could say:

return -1 * child1.Age.CompareTo(child2.Age);

You can get it even simpler. Because -(-1) = 1, and -(1) = -1, your Compare() method becomes:

return -child1.Age.CompareTo(child2.Age);

Not only no elses, but also no ifs! Very pretty.

Reducing the number of paths through your code (i.e., reducing cyclomatic complexity) gets it closer to being read like prose. Simpler code has fewer bugs, and your successors who have to read your code will think you are smart and good looking. Keep a healthy mistrust of else statements, and write pretty code.

Dictionary as DropDownList Data Source

I have a Dictionary<T,T> called myListOptions. To use it as the datasource for an ASP.NET DropDownList called MyList...

MyList.DataSource = myListOptions;
MyList.DataTextField = "Value";
MyList.DataValueField = "Key";
MyList.DataBind();

It took me some rummaging to realize that you use the actual strings "Value" and "Key," so I hope this post comes in handy for someone else in the future (even if it's me).

The elegance of Queues and Stacks

I've been meditating on the beauty of Queues and Stacks. Like Lists, they are collections of objects, but with their own special attributes.

Queues are like a line of customers: the first one in line is the first one you'll serve (FIFO, First In First Out). These could be used for queued up messages that you have to process in order.

Stacks are like a deck of playing cards: the last one you lay on the top will be the first one you draw off the top (LIFO, Last In First Out). These could be used for tracking a sequence of actions that you might need to undo.

You can Peek() at the top item in a Queue or a Stack, but you can't reference a specific item by index. You have access only to the First In or Last In (respectively), and can only add things to the next spot in line.

I like how Queue.Dequeue() and Stack.Pop() both return the next object and remove it from the collection. It seems tidy to do it all in one method. But you're not stuck with that; like I said, you can still Peek() if you just want to see what's next, but leave the object in place.

If you need to deal with your objects in an order relating to how you encountered them, as opposed to sorting by some attribute of the object, then a Queue or a Stack might be a better fit than a List.

Fail Fast, and Mind the Outs

I had a learning opportunity on two coding concepts (in C#) yesterday:

• out parameters are tricksy and not to be trusted.
• Fail fast. When the world starts getting uncertain and hairy, throw an exception and get out of there.
  

The setup: I'm retrieving app configuration values from the database, a minimum and maximum allowable threshold. The db table is generic, containing key/value pairs for many purposes, so the values are stored as varchars, but I want integers here. If I don't get a value, I have some defaults we should use. The colleague who would be reviewing my code has declared he is allergic to nested if/else blocks, so I need a direct path through this logic.

Too crufty: One might be tempted to think about the logic this way. If I get a value from the database, but it isn't an int, set it to the default; if I don't get a value from the database, set it to the default. When you say it in English, you can hear the repeated code, violating the DRY principle. It's a small repetition, and in a very localized piece of code, but still icky.

Tripping over outs: Since I didn't want to repeat that default value, and I wasn't happy with those elses, I tried this. The default value is 100; if you get a different integer from the database, use that. But I didn't get it quite right:

[using a data reader]
int max = 100;
if (dataReader.Read())
{
   Int32.TryParse(dataReader.GetString(0), out max); 
     //This is wrong.
}
return max;

Can you see what I was thinking, though? "If you successfully parse the result into an integer, set 'max' to that value. Otherwise, leave it alone." That was my mistake. What actually happens is, if TryParse can't parse it into an integer, it makes it undefined. From an msdn article, "The value of an out argument will not be passed to the out parameter."

What I really needed to do there is, "if not int.tryparse, then set it to my default value." I should check the boolean return value whenever I use TryParse.

Hold on, cowboy: But what does it mean if the configuration value in my database isn't an integer? Somebody typed something screwy. And they probably did it by updating the database, not using the admin screen. The world is in an unknown state, society is breaking down—cats and dogs, living together...

I don't want to set any default value. I want to throw an exception and get the heck out of there. So the final version is:

[using a data reader]
int max = 100;
if (dataReader.Read())
{
   max = Int32.Parse(dataReader.GetString(0)); 
     //Fail fast and make your escape.
}
return max;

This achieves three design principles, which is why I wanted to write it down and reinforce my learning:

1. Find the most direct path through the logic. Elses are a smell.
2. Don't repeat yourself. Even though the repeating here was trivial, it caught my attention and made me think through what should really happen, which led me to...
3. When you start getting unexpected behavior, hiding it and swallowing it will cause worse problems. If you get a "that can't happen" scenario, throw an exception.