Code navigation – quick tip

When downloading open-source projects, tutorials and other code, there is an initial reaction:

  • This code is really easy to digest
  • Wow, let me dig in and see what is here!

Obviously, the reader would appreciate the first experience.  Here is one tip on how to turn a bad navigation experience into a good one.  This is not the only factor, but it is one I have experienced. 

This code is very simple in its own right, and it doesn’t take long to fully digest this class, but there are some improvements that can be made, and this file came from a very useful tutorial.

Let’s start with a simple class for a GuestBook app:

   1:  // ----------------------------------------------------------------------------------
   2:  // Title, something, something, something
   3:  // 
   4:  // Copyright (c) Some Company. LICENSE TITLE
   5:  // 
   6:  // THIS CODE AND INFORMATION ARE PROVIDED "AS IS" WITHOUT WARRANTY OF ANY KIND, 
   7:  // EITHER EXPRESSED OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE IMPLIED WARRANTIES 
   8:  // OF MERCHANTABILITY AND/OR FITNESS FOR A PARTICULAR PURPOSE.
   9:  // ----------------------------------------------------------------------------------
  10:  // The example companies, organizations, products, domain names,
  11:  // e-mail addresses, logos, people, places, and events depicted
  12:  // herein are fictitious.  No association with any real company,
  13:  // organization, product, domain name, email address, logo, person,
  14:  // places, or events is intended or should be inferred.
  15:  // ----------------------------------------------------------------------------------
  16:   
  17:  using System;
  18:  using System.Collections.Generic;
  19:  using System.Linq;
  20:  using System.Text;
  21:   
  22:  namespace GuestBook_Data
  23:  {
  24:      public class GuestBookEntry
  25:          : Microsoft.WindowsAzure.StorageClient.TableServiceEntity
  26:      {
  27:          public GuestBookEntry()
  28:          {
  29:              PartitionKey = DateTime.UtcNow.ToString("MMddyyyy");
  30:   
  31:              // Row key allows sorting, so we make sure the rows come back in time order.
  32:              RowKey = string.Format("{0:10}_{1}", DateTime.MaxValue.Ticks - DateTime.Now.Ticks, Guid.NewGuid());
  33:          }
  34:   
  35:          public string Message { get; set; }
  36:          public string GuestName { get; set; }
  37:          public string PhotoUrl { get; set; }
  38:          public string ThumbnailUrl { get; set; }
  39:      }
  40:  }

I’ll outline some issues with this code file besides my partial violation of the license.

  1. The auto properties are not fully visible without scrolling.  For a class as small as this, the most meaningful information is hidden until the reader scrolls down.
  2. The least important information is at the very top of the file (license information).  The license is at the package level, and the reader already knows the license before diving into each individual file.
  3. The structure of the class is de-emphasized.  The properties of the class are the basic structure, and they are at the end.

I will refactor this class and present what I believe to be a more easily digestible version.  Here goes:

   1:  using System;
   2:   
   3:  namespace GuestBook_Data
   4:  {
   5:      public class GuestBookEntry
   6:          : Microsoft.WindowsAzure.StorageClient.TableServiceEntity
   7:      {
   8:          public string Message { get; set; }
   9:          public string GuestName { get; set; }
  10:          public string PhotoUrl { get; set; }
  11:          public string ThumbnailUrl { get; set; }
  12:   
  13:          public GuestBookEntry()
  14:          {
  15:              PartitionKey = DateTime.UtcNow.ToString("MMddyyyy");
  16:   
  17:              // Row key allows sorting, so we make sure the rows come back in time order.
  18:              RowKey = string.Format("{0:10}_{1}", DateTime.MaxValue.Ticks - DateTime.Now.Ticks, Guid.NewGuid());
  19:          }
  20:      }
  21:  }
  22:   
  23:  // ----------------------------------------------------------------------------------
  24:  // Title, something, something, something
  25:  // 
  26:  // Copyright (c) Some Company. LICENSE TITLE
  27:  // 
  28:  // THIS CODE AND INFORMATION ARE PROVIDED "AS IS" WITHOUT WARRANTY OF ANY KIND, 
  29:  // EITHER EXPRESSED OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE IMPLIED WARRANTIES 
  30:  // OF MERCHANTABILITY AND/OR FITNESS FOR A PARTICULAR PURPOSE.
  31:  // ----------------------------------------------------------------------------------
  32:  // The example companies, organizations, products, domain names,
  33:  // e-mail addresses, logos, people, places, and events depicted
  34:  // herein are fictitious.  No association with any real company,
  35:  // organization, product, domain name, email address, logo, person,
  36:  // places, or events is intended or should be inferred.
  37:  // ----------------------------------------------------------------------------------

Here are the improvements:

  • Using statements have been minimized to remove the unneeded.
  • Properties are first to emphasize class structure.
  • License is at the bottom.
  • The entire class code is visible without scrolling – the license falls off the fold instead.

This has been a quick tip.  Hopefully you will benefit from this.  I haven’t always thought about factors like this in software, but readability is very important.  Code will be read MANY more times than it is written or modified.

Comments

Jonathan Christian said on 1.19.2011 at 5:11 PM

Good ideas, although some people will probably have qualms about moving the license to the bottom of code files.

Also, if you are like me and always forget to sort and remove using statements, the power tools for VS have an option to 'Remove and Sort Usings on save'. Pretty useful imho.

Blake Niemyjski said on 1.19.2011 at 5:42 PM

You could also just use regions. ReSharper and the StyleCop addin make this a breeze.

Christopher Bennage said on 1.19.2011 at 6:16 PM

We spend a lot of time of this sort of thing. I've also been converted to nesting my using statements inside the namespace as it removes some redundancy.

I must also confess that regions do not help readability for me. Ever. Perhaps one to hide the license, but even then it would take serious convincing.

Daniel Bullington said on 1.19.2011 at 8:42 PM

I leverage Resharper's member reoganization features. See the following blog article:

blog.softwareishardwork.com/.../source-code-nor

Leom Burke said on 1.20.2011 at 3:39 AM

This is more readable but it is not compliant with the default StyleCop rules. We use the default rules so would be refused check-in with this class - other than the obvious commenting issues, the usings should be inside the namespace and the properties should follow the constructor not the other way around.

David Mohundro said on 1.20.2011 at 9:33 AM

To be honest, I'm occasionally moving properties/fields BELOW methods. I find that I'm usually more interested in behavior than in state. However, given your example, I'd keep the properties on top because they're more important in this example. Guess it depends on the class.

Simon said on 1.21.2011 at 12:04 AM

Why bother with a license on every file?