You have reached the blog of Keith Elder. Thank you for visiting! Feel free to click the twitter icon to the right and follow me on twitter.

Cleaning Up Your C# Closet, Making Messy C# Code More Readable

Posted by Keith Elder | Posted in C# | Posted on 22-12-2008

How Clean is Your Closet?


Have you ever been instructed to make an enhancement to an existing project where the codebase looked like the picture above?  I have.  As a matter of fact, I’ve done my fair share of creating messy code just like that.  I’m not saying I’m proud of admitting that but as they say, “The truth hurts”.

Let’s pretend for a moment this is my closet and you were visiting.  We were about to leave to go to town and you said, “Brrrr, it is cold outside, do you have a scarf?”.  I said, “Sure, it is in the closet, there is a purple one in there.”. 

You then stumble into the closet to find the scarf.  How long do you think it would take you?  The likelihood of you finding it within a few seconds is slim to none. 

Let’s switch gears to programming.  How long would it take to implement a task if the picture above represented the codebase of an existing project?  A long time for sure, definitely longer than it should take no doubt.  Why?  Because things that are messy take longer!  Here is an Elderism (common sense words of wisdom to live by) that sums up messy projects:

ELDERISM – “A messy project takes longer to understand and will only get messier.”

Switch gears for a moment and let’s say you needed to find the purple scarf in this closet.  Obviously since things are organized and well laid out, it becomes easy to locate, add, and remove things. 

image The really sad part about this whole analogy is that for some reason, a lot of developers are happy with wallowing in a perfect mess.  Not only does this make things hard for them but for other developers on the team as well. 

A Real World Mess

I was reminded of this the other day when I started to work on Witty Twitter, an open source WPF Twitter client several of us work on.  I hadn’t worked on the project very much lately but I wanted to take some time during vacation to clean some things up and add some new features.  I know how most things work in codebase but the biggest problem with the codebase is the lack of structure and organization to it.  I started to implement a new feature and suddenly I realized I was contributing to the code rot.  Remember the Elderism, “A  messy project takes longer to understand and will only get messier.”?  Well, here I was adding more mess to the codebase to get something to work. 

Finally I said, enough is enough.  My contribution is going to be to simplify and clean up this code.  In other words, clean up and organize the closet.  To give you an idea of the mess staring me in the face, here is the constructor of the main application.  This is ONE function which initializes the main form Witty uses.

   1: public MainWindow()
   2: {
   3:     this.InitializeComponent();
   5: #if DEBUG
   6:     Title = Title + " Debug";
   7: #endif
   9:     // Trap unhandled exceptions
  10:     LayoutRoot.Dispatcher.UnhandledException += new DispatcherUnhandledExceptionEventHandler(Dispatcher_UnhandledException);
  12:     #region Minimize to tray setup
  14:     _notifyIcon = new System.Windows.Forms.NotifyIcon();
  15:     _notifyIcon.BalloonTipText = "Right-click for more options";
  16:     _notifyIcon.BalloonTipTitle = "Witty";
  17:     _notifyIcon.Text = "Witty - The WPF Twitter Client";
  18:     _notifyIcon.Icon = Witty.Properties.Resources.AppIcon;
  19:     _notifyIcon.DoubleClick += new EventHandler(m_notifyIcon_Click);
  21:     System.Windows.Forms.ContextMenu notifyMenu = new System.Windows.Forms.ContextMenu();
  22:     System.Windows.Forms.MenuItem openMenuItem = new System.Windows.Forms.MenuItem();
  23:     System.Windows.Forms.MenuItem exitMenuItem = new System.Windows.Forms.MenuItem();
  25:     notifyMenu.MenuItems.AddRange(new System.Windows.Forms.MenuItem[] { openMenuItem, exitMenuItem });
  26:     openMenuItem.Index = 0;
  27:     openMenuItem.Text = "Open";
  28:     openMenuItem.Click += new EventHandler(openMenuItem_Click);
  29:     exitMenuItem.Index = 1;
  30:     exitMenuItem.Text = "Exit";
  31:     exitMenuItem.Click += new EventHandler(exitMenuItem_Click);
  33:     _notifyIcon.ContextMenu = notifyMenu;
  34:     this.Closed += new EventHandler(OnClosed);
  35:     this.StateChanged += new EventHandler(OnStateChanged);
  36:     this.IsVisibleChanged += new DependencyPropertyChangedEventHandler(OnIsVisibleChanged);
  38:     // used to override closings and minimize instead
  39:     this.Closing += new CancelEventHandler(MainWindow_Closing);
  41:     #endregion
  43:     #region Single instance setup
  44:     // Enforce single instance for release mode
  45: #if !DEBUG
  46:     Application.Current.Exit += new ExitEventHandler(Current_Exit);
  47:     _instanceManager = new SingleInstanceManager(this, ShowApplication);
  48: #endif
  49:     #endregion
  51:     // Set the data context for all of the tabs
  52:     LayoutRoot.DataContext = tweets;
  53:     RepliesListBox.ItemsSource = replies;
  54:     UserTab.DataContext = userTweets;
  55:     MessagesListBox.ItemsSource = messages;
  57:     // Set how often to get updates from Twitter
  58:     refreshInterval = new TimeSpan(0, int.Parse(AppSettings.RefreshInterval), 0);
  60:     this.Topmost = AlwaysOnTopMenuItem.IsChecked = AppSettings.AlwaysOnTop;
  62:     // Does the user need to login?
  63:     if (string.IsNullOrEmpty(AppSettings.Username))
  64:     {
  65:         PlayStoryboard("ShowLogin");
  66:     }
  67:     else
  68:     {
  69:         LoginControl.Visibility = Visibility.Hidden;
  71:         System.Security.SecureString password = TwitterNet.DecryptString(AppSettings.Password);
  73:         // Jason Follas: Reworked Web Proxy - don't need to explicitly pass into TwitterNet ctor
  74:         //twitter = new TwitterNet(AppSettings.Username, password, WebProxyHelper.GetConfiguredWebProxy());
  75:         twitter = new TwitterNet(AppSettings.Username, password);
  77:         // Jason Follas: Twitter proxy servers, anyone?  (Network Nazis who block twitter.com annoy me)
  78:         twitter.TwitterServerUrl = AppSettings.TwitterHost;
  80:         // Let the user know what's going on
  81:         StatusTextBlock.Text = Properties.Resources.TryLogin;
  82:         PlayStoryboard("Fetching");
  84:         // Create a Dispatcher to attempt login on new thread
  85:         NoArgDelegate loginFetcher = new NoArgDelegate(this.TryLogin);
  86:         loginFetcher.BeginInvoke(null, null);
  87:     }
  89:     InitializeClickOnceTimer();
  91:     InitializeSoundPlayer();
  93:     ScrollViewer.SetCanContentScroll(TweetsListBox, !AppSettings.SmoothScrolling);
  95:     //Register with Snarl if available
  96:     if (SnarlConnector.GetSnarlWindow().ToInt32() != 0)
  97:     {
  98:         //We Create a Message Only window for communication
 100:         this.SnarlConfighWnd = Win32.CreateWindowEx(0, "Message", null, 0, 0, 0, 0, 0, new IntPtr(Win32.HWND_MESSAGE), IntPtr.Zero, IntPtr.Zero, IntPtr.Zero);
 101:         WindowsMessage wndMsg = new WindowsMessage();
 102:         SnarlConnector.RegisterConfig(this.SnarlConfighWnd,"Witty",wndMsg);
 104:         SnarlConnector.RegisterAlert("Witty", "New tweet");
 105:         SnarlConnector.RegisterAlert("Witty", "New tweets summarized");
 106:         SnarlConnector.RegisterAlert("Witty", "New reply");
 107:         SnarlConnector.RegisterAlert("Witty", "New direct message");
 109:     }
 110: }

In case you don’t scroll down and actually glance at the code, there are over 100 lines of code for this constructor.  No matter how smart you are, there is no way one can read that code and fully understand “what” it is doing without devoting an enormous amount of time to it.  Everything in there, with the exception of a few functions that get called, the code is all about “how”.  All of the code in that constructor is needed too.  It isn’t like it can be taken out, it must be there to set things up in the application. 

Cleaning Up The Closet

If you don’t take anything else away from this article remember this one fact.  Developers reading your code rarely are worried about the “how”, they need to know the “what” so they can figure out where to put their code.  This concept is earth shatteringly simple, yet as you can see with the above code, we have a perfect real world example of a “how” that has turned itself into a mess. 

Code Smell #87 – Comments

A code smell is something we refer to in the business as something that doesn’t look right, or that shouldn’t be done that way, or something that will lead the developer down a dark path.  It means many things and that’s why I labeled this as #87. 

Comments in code have saved many developers endless hours, but they have also cost many developers thousands of hours.  A rule I have, is code shouldn’t have any comments in the code.  My main reason for this is refactoring. 

Code refactoring is the process of changing a computer program‘s code to make it amenable to change, improve its readability, or simplify its structure, while preserving its existing functionality.

In software development we constantly refactor our code.  Guess what happens to those comments though?  They don’t get refactored! 

A developer can completely change how a program works and those nasty comments are still there.  Then along comes someone else who reads the comment and thinks the code is broken because it doesn’t do what the comment said it was doing.  They then refactor it back and things break.

The best thing to do is not comment your code.  If you have the urge to put a comment on something, that means you didn’t express the intent of the code clearly enough, thus it is a code smell.  Let’s look at a few examples from our example above to see how this plays out.

Let’s start with line #95 in the example above.  Here is that example:

//Register with Snarl if available
if (SnarlConnector.GetSnarlWindow().ToInt32() != 0)
    //We Create a Message Only window for communication
    this.SnarlConfighWnd = Win32.CreateWindowEx(0, "Message", null, 0, 0, 0, 0, 0, new IntPtr(Win32.HWND_MESSAGE), IntPtr.Zero, IntPtr.Zero, IntPtr.Zero);
    WindowsMessage wndMsg = new WindowsMessage();
    SnarlConnector.RegisterConfig(this.SnarlConfighWnd, "Witty", wndMsg);
    SnarlConnector.RegisterAlert("Witty", "New tweet");
    SnarlConnector.RegisterAlert("Witty", "New tweets summarized");
    SnarlConnector.RegisterAlert("Witty", "New reply");
    SnarlConnector.RegisterAlert("Witty", "New direct message");

Notice the comment in the first line of the code?  The comment is telling us “what” the if block is doing.  Even though I can read this:

(SnarlConnector.GetSnarlWindow().ToInt32() != 0)

I honestly do not know the Snarl API and thus I have no idea “what” that is doing, or why.  In this case, the developer that wrote this in their mind said this was something they had to figure out “how to do” so they wanted to leave a bread crumb here to tell everyone “what it’s doing”.

Now think for a second.  Without using a comment, how else could they have told us the exact same thing?  The answer: CODE. 

If a developer focuses on making their code more readable the end result will be much neater and it will also express the “what it is doing”.  This means other developers can move quickly through the project / closet to figure out where things are.

For those cleaning up code like this, more times than not you can just take the comments and turn that into functions.  This is the case so much in fact that Visual Studio plug-ins like Refactor! Pro automatically turn comments like that into the name of the function when extracting code into a method. 

In this example there are two comments.  We’d refactor this to read like this:

public MainWindow()
private void RegisterWithSnarlIfAvailable()
    if (SnarlConnector.GetSnarlWindow().ToInt32() != 0)
private void CreateSnarlMessageWindowForCommunication()
    this.SnarlConfighWnd = Win32.CreateWindowEx(0, "Message", null, 0, 0, 0, 0, 0, new IntPtr(Win32.HWND_MESSAGE), IntPtr.Zero, IntPtr.Zero, IntPtr.Zero);
    WindowsMessage wndMsg = new WindowsMessage();
    SnarlConnector.RegisterConfig(this.SnarlConfighWnd, "Witty", wndMsg);
    SnarlConnector.RegisterAlert("Witty", "New tweet");
    SnarlConnector.RegisterAlert("Witty", "New tweets summarized");
    SnarlConnector.RegisterAlert("Witty", "New reply");
    SnarlConnector.RegisterAlert("Witty", "New direct message");

With a simple refactor we have fully expressed the “what” the code is doing and made it more readable. 

Applying these simple concepts to the original example that was over 100 lines long, we can refactor this into readable code with zero comments that expresses what is going on instead of how.  The end result is less code to read in the constructor that reads more like a story describing what the constructor is doing. 

   1: public MainWindow()
   2: {
   3:     this.InitializeComponent();
   5:     TrapUnhandledExceptions();
   7:     SetupNotifyIcon();
   9:     SetupSingleInstance();
  11:     SetDataContextForAllOfTabs();
  13:     SetHowOftenToGetUpdatesFromTwitter();
  15:     InitializeClickOnceTimer();
  17:     InitializeSoundPlayer();
  19:     InitializeMiscSettings();
  21:     RegisterWithSnarlIfAvailable();
  23:     DisplayLoginIfUserNotLoggedIn();
  24: }

Do you see the difference?  It is night and day isn’t it?  There is no more wondering, guessing, and also worrying about doing something in the wrong place. 

If you are dealing with messy closets, I mean code, apply these basic principles and you’ll wind up with codebases that are more readable and easier to refactor down the road.  This is only one thing you can do to clean up messy projects.  There are others but we have to start somewhere. 

Once things are refactored so they express the what things will get more clear and other refactorings will start to bubble up.  For example, if you look at the final code above, you’ll notice the DisplayLoginIfUserNotLoggedIn() method is called last in the constructor.  This makes sense.  We should call the login screen after everything else is initialized.  However, in the original code it wasn’t last.  How would I have figured that out if I hadn’t made the code more readable?  Exactly my point!

It is a very powerful concept, give it try.

Comments (10)

Too true on the comments. I have written a similar blog post about them. I have even put together this poster that I proudly display in mu cube at work:



Read my comments above on XML comments.

Ok I see how your code will get cleaner. Not 100s of lines in a single method. But no comments..what’s wrong with XML commenting?

Do you know of a good book that talks more about this topic?

Kind regards,


I’m with the others, I would have taken the:
SnarlConnector.GetSnarlWindow().ToInt32() != 0
and probably just do a bool beforehand:

bool snarlIsAvailable = SnarlConnector.GetSnarlWindow().ToInt32() != 0;
if(snarlIsAvailable) {…}

This way, I don’t need any comments. The only time I comment anymore is when I need to explain the “why” of what was done mainly for future refactorings to take into account.

You’re right, it takes regular effort to keep things tidy or the closet falls into disrepair.

I recently wrote about this on my blog using the Boy Scout adage of “always leave [your code] a little better than you found it.” It was one of the best gems from Bob Martin’s “Clean Code” book.


Thanks for reminding me of that, great point that I left out of the article.

Yes, comments on that method in XML are perfectly OK in my book and is where, if one feels the need to comment something, where it should go.

When I say no comments, let me clarify that I mean between the curly braces.


I still don’t think all comments are bad.
For example, for the method
bool IsSnarlAvailable() {
return SnarlConnector.GetSnarlWindow().ToInt() != 0;

I’d put some XML comment on that top saying that it returns a boolean value meaning whether or not the SnarlWindow is available (I know, it’s the same as the name except more verbose)
And a comment saying why this int would be 0 and GetSnarlWindow() wouldn’t return null for instance (Like you, I don’t know and don’t want to know the Snarl interface – I just want to know why they did SnarlConnector.GetSnarlWindow().ToInt() != 0.

The ‘What’ should be expressed in the method name, and more verbosely in the XML comment.
The ‘How’ should be expressed in the actual code in the (inner) method.
And the ‘Why’ should be in a comment. Not why I’m doing this – but why I’m doing it this way.


You are on the right track sir! Once you get the larger pieces in place you keep going. I just didn’t take the time to do that in the article.


Good advice, but I still don’t see what the following code is doing very easilly.

SnarlConnector.GetSnarlWindow().ToInt32() != 0

I’d prefer to see another method called IsSnarlAvailable() that returns the result of the previous code.

Great post. I have certainly contributed to my share of messy closets and have cleaned up after others. What is not so easy is figuring out how to do the cleaning when you have outside entities which may rely on things being located in a certain spot.

Write a comment