Showing posts with label SOLID. Show all posts
Showing posts with label SOLID. Show all posts

Saturday, July 11, 2009

Chaining the C# Null Coalescing Operator

This is something that came up in the comments last week.  I was refactoring some code and wound up with the  accessor method below that performs multiple null checks while trying to assign a value to _currentUser.

// CurrentUser

 private WebUser _currentUser;

 public WebUser CurrentUser

{

   get

   {

     if (_currentUser == null) _currentUser = GetWebUserFromSession();

     if (_currentUser == null) _currentUser = GetWebUserFromTrackingCookie();

     if (_currentUser == null) _currentUser = CreateNewWebUser();

     return _currentUser;

   }

}

Now this code is pretty clear, but a reader named Brian pointed out in a comment that we could shorten the code a bit by using the Null Coalescing operator “??”.  If you haven’t used the Null Coalescing operator yet, check it out.  It’s great for data access code where a method may return data or a null value.  The basic syntax is illustrated in this block of code from msdn:

   // y = x, unless x is null, in which case y = -1.
   int y = x ?? -1;

   // Assign i to return value of method, unless
   // return value is null, in which case assign
   // default value of int to i.
   int i = GetNullableInt() ?? default(int);

So, this is really cool, but there’s more. What Brian pointed out was that you can chain the ?? operator to do multiple null checks.  So, my block of code above can be rewritten like this:

// CurrentUser

 private WebUser _currentUser;

 public WebUser CurrentUser

{

   get

   {

     _currentUser = _currentUser ??

                    GetWebUserFromSession() ??

                    GetWebUserFromTrackingCookie() ??

                    CreateNewWebUser();

     return _currentUser;

   }

}

Note that normally the _currentUser assignment would fit all on one line but due to the width limitation of my blog I broke it up into multiple lines.  So, C# Null Coalescing operator chaining. I like it. I’ll be adding it to my toolbox.

 

Addendum:

Hey, you guys pretty much hated this change to the code. I got no blog comments but I received quite a few emails about this technique and the prevailing opinion seems to be that the original syntax with the multiple if blocks was clearer.  In fact, not a single person who emailed me liked the ?? method. So I took another look and I think I agree.  The original code does seem a little clearer to me and I don’t really think I was able to make my code any shorter or simpler by using ??.  So this may not have been the best example, but I do still think this is a very cool technique for the right situation. As always, I just need to make sure that I’m refactoring because the changes make the code cleaner, not just because they’re clever.

Friday, July 3, 2009

SOLID C# Code: Smaller Methods == Clean Code?

I’m a big fan of Robert Martin.  His book on Agile Patterns in C# is still one of the three most important programming books I’ve ever read.  If you listen to Uncle Bob for any amount of time, it won’t be long before you start hearing terms like SOLID Principles and Clean Code.  These are concepts that are widely known, but still a bit tough to define.  What exactly is clean code?  What does SOLID code look like?

We’ll I’m not smart enough to provide a definition that’s any better than what’s already out there, but I can point at an example and say “that smells pretty SOLID”.  So, below is an example of some code that I wrote for an ASP.Net MVC application.  CurrentUser is a property that wraps an instance of my WebUser class, which represents, you guessed it, the currently logged in user for my web app. When the CurrentUser property returns a user, there are four possible places that it might have to check to find that user:

  1. There may already be a WebUser instance in the private member _currentUser,
  2. There may be a WebUser instance stored in the Session,
  3. There may be a TrackingCookie in the HTTP Request that can be used to get an existing WebUser,
  4. We may have none of the above, in which case we have to create a new WebUser.

So, with that said, let’s take a look at my first version of this code.

// CurrentUser

private WebUser _currentUser;

public WebUser CurrentUser

{

    get

    {

        // Do we already have the CurrentUser?

        if (_currentUser == null)

        {

            // Try to get the user from Session

            Object obj  = HttpContext.Current.Session["__currentUser"];

            if (obj != null)

            {

                _currentUser = (WebUser)obj;

                return _currentUser;

            }

            // Try to get the user from a TrackingCookie

            SecurityHelper secHelper = new SecurityHelper();

            WebUserRepository rep = new WebUserRepository();

            if (secHelper.TrackingGuid != Guid.Empty)

            {                      

                _currentUser = rep.GetWebUserByTrackingGuid(secHelper.TrackingGuid);

                if (_currentUser != null) return _currentUser;

            }

            // If we still don't have a user then we need to create a new

            // WebUser with a new TrackingGuid.

            WebUserFactory factory = new WebUserFactory();

            _currentUser = factory.CreateWebUser();

        }

        return _currentUser;

    }

}

 

 

Hmmmmmm. Not too horrible, not too long and ungainly, but it definitely has some code smell (the bad kind).  First, I’m not thrilled about the multiple returns nested in conditional blocks. Also, the code is doing a number of different things that I felt needed comments to explain.  Now I love comments, and I’m a firm believer that when in doubt, comment.  But I’ve also come to realize that it is possible to design code in such a way that it can be just as clear, and just as understandable, without the need for comments. 

So how do we reach this promised land of understandability? The main technique that I’ve been using is simply to take my big blocks of code that do several different things, and break them up into several separate, clearly named methods, and then just call them from my main block. Here’s how I applied this technique to my CurrentUser property.

 

 

// CurrentUser

 private WebUser _currentUser;

 public WebUser CurrentUser

{

   get

   {

     if (_currentUser == null) _currentUser = GetWebUserFromSession();

     if (_currentUser == null) _currentUser = GetWebUserFromTrackingCookie();

     if (_currentUser == null) _currentUser = CreateNewWebUser();

     return _currentUser;

   }

}

 

 

// GetWebUserFromSession

private WebUser GetWebUserFromSession()

{

     Object obj = HttpContext.Current.Session["__currentUser"];

     return obj == null ? null : (WebUser)obj;

}

 

// GetWebUserFromTrackingCookie

private WebUser GetWebUserFromTrackingCookie()

{

     SecurityHelper secHelper = new SecurityHelper();

     WebUserRepository rep = new WebUserRepository();

     if (secHelper.TrackingGuid == Guid.Empty)

         return null;

     else

         return rep.GetWebUserByTrackingGuid(secHelper.TrackingGuid);

}

 

// CreateNewWebUser

private WebUser CreateNewWebUser()

{

     WebUserFactory factory = new WebUserFactory();

     return factory.CreateWebUser();

}

 

Now I realize that in this second version I wrote more code than the first, and I do a couple of extra null checks in my main property code, but look at how easily understandable everything is.  I mean really. There’s not a single comment, but can you imagine anybody not understanding exactly what’s going on in this block?

   get

   {

     if (_currentUser == null) _currentUser = GetWebUserFromSession();

     if (_currentUser == null)  _currentUser = GetWebUserFromTrackingCookie();

     if (_currentUser == null)  _currentUser = CreateNewWebUser();

     return _currentUser;

   }

Plus, because I factored out methods like GetWebUserFromSession() and GetWebUserFromTrackingCookie() I can now use those methods in other parts of my class without having to rewrite the functionality.  So overall, I think this version smells much more SOLID.  What do you think?  If anyone has ideas or favorite techniques for how to get more SOLID, please leave a comment.