Don't hide the ugly

I wanted to take some time to highlight the difference between encapsulation and subterfuge.  Just so we're on the same page:

• Encapsulation: The ability to provide users with a well-defined interface to a set of functions in a way which hides their internal workings.
• Subterfuge: An artifice or expedient used to evade a rule, escape a consequence, hide something, etc.

When related to code, both of these techniques hide internal details of the system.  The key difference is who we're hiding the details from.  With encapsulation, we're hiding details from end-users or consumers of our API, which is a good thing.  With subterfuge, we're hiding ugly from developers needing to change the API, which can be disastrous.

Subterfuge hides the ugly, and for ugly to get fixed, I want it front and center.  Subterfuge comes in many varieties, but all achieve the same end result of hiding the ugly instead of fixing the ugly.

Region directives

Region directives in C# and VB.Net let you declare a region of code that can be collapsed or hidden.  I've used them in the past to partition a class based on visibility or structure, so I'll have a ".ctor" region or maybe a "Public Members" region.  Other regions can be collapsed so I don't need to look at them.

For example, our DataGateway class seems nice and concise, and it looks like it has only about 10-20 lines of code:

One small problem, note the region "Legacy db access code".  By applying a region to some nasty code, I've hidden the ugliness away from the developer who otherwise might think it was a problem.  The developer doesn't know about the problem, as I've collapsed over 5000 lines of code into one small block.

If a class is big and nasty, then just let it all hang out.  If it's really bothersome, extract the bad code into a different class.  At least you'll have separation between ugly and nice.  Hiding ugly code in region directives doesn't encourage anyone to fix it, and it tends to deceive those glancing at a class how crazy it might be underneath.

One type per file, please

Nothing irks me more than looking at a solution in the solution explorer, seeing a relatively small number of files, and then realizing that each file has a zillion types.  A file called "DataGateway.cs" hides all sorts of nuttiness:

public enum DataGatewayType
{

}

public abstract class DataGateway
{
    public abstract int[] GetCustomerIDs();
}

public class SqlDataGateway : DataGateway
{
    public override int[] GetCustomerIDs()
    {
        // sproc's for all!!!

        return new int[] { };
    }
}

public class OracleDataGateway : DataGateway
{
    public override int[] GetCustomerIDs()
    {
        // now we're getting enterprise-y 

        return new int[] { };
    }
}

public class MySqlGateway : DataGateway
{
    public override int[] GetCustomerIDs()
    {
        // we don't support hippies 

        return null;
    }
}

Java, as I understand it, has a strict convention of one type per file.  I really like that convention (if I'm correct), as it forces the developer to match their file structure to their package (assembly) structure.  No such luck in .NET, although there may be some FxCop or ReSharper rules to generate warnings.

The problem with combining multiple types to one file is that it makes it very difficult for the developer to gain any understanding of the code they're working with.  Incorrect assumptions start to arise when I see a file name that doesn't match the internal structure.  Some of the rules I use are:

• One type per file (enum, delegate, class, interface, struct)
• Project name matches assembly name (or root namespace name)
• File name matches type name
• Folder structure matches namespaces
  • i.e. <root>\Security == RootNamespace.Security

With these straightforward rules, a developer can look at the folder structure or the solution explorer and know exactly what types are in the project, how they are organized, and maybe even understand responsibilities and architecture.  Even slight deviations from the above rules can cause unnecessary confusion for developers.  You're not doing anyone any favors by cramming 10 classes into one file, so don't do it.

Letting it all hang out

When I was young, my mom would insist on me cleaning my room once a week (the horror!).  Being the resourceful chap that I was, I found I could get the room resembling clean by cramming everything just out of sight, into the closet, under the bed, even in the bed.  This strategy worked great until I actually needed something I stashed away, and couldn't find it.  I had created the illusion of organization, but the monster was lurking just out of sight.

So instead of hiding the problem, I forced myself to deal with the mess until I cared enough to clean it up.  Eventually this led me to fix problems early and often, so they don't snowball into a disastrous mess.  This wasn't out of self-satisfaction or anything like that, but laziness, as I found it was more work to deal with hidden messes than it was to clean it up in the first place.

If my code is a mess, I'll just let it be ugly.  Ugly gets annoying and ugly gets fixed, but ugly swept under the rug gets overlooked, until it pounces on the next unsuspecting developer.