A few months ago I blogged about how we're using the single responsibility principle in our application at work.  I'm going to make an effort here to write more "Real Life" content.  Often when a developer read principles or ideas in books, there is disconnect between what he/she is reading and how an implementation would look in the real world.  The goal of these "Real Life" scenarios is to show areas where I feel I've reaped some benefit from following a specific pattern or practice.  This does not mean that my implementation is the only way or always "book correct".  The goal is to act as a bridge (quite possibly a old rickety bridge) between the book knowledge and the real world.

Moving along...

One of the purported benefits of OOP has always been the abundant reuse of code.  However, in my experience many applications fail to realize any amount of reuse beyond copying and pasting between applications.  To really take advantage of code reuse you have to have small, concise pieces which you can rearrange in order to make a new behavior.  As a side note, and I want this be clear, many of the principles of good software development, ie. SOLID, go hand-in-hand.  You can't do one without the other and the benefits of one cannot be fully realized without the implementation of the others.  You'll notice that a few sentences ago I said "small, concise pieces".  A light bulb may have gone off in your head thinking "Single Responsibility Principle", and if it did, bonus points for you.

The key to code reuse is to build your components in a very focused way.  By doing so, you can add functionality by combining different pieces of you application.

Consider the following code used to authenticate a user into a site:

   1: protected void btnSubmit_Click(object sender, EventArgs e)

   2: {

   3:     string Username = Server.HtmlEncode(txtUsername.Text);

   4:     string Password = Server.HtmlEncode(txtPassword.Text);

   5:     int nUserID;

   6:  

   7:     if (Username != "" && Password != "")

   8:     {

   9:         using (SqlConnection cnn = new SqlConnection(connectionString))

  10:         {

  11:             using (SqlCommand cmd = cnn.CreateCommand())

  12:             {

  13:                 cmd.CommandText = "usp_Login";

  14:                 cmd.Parameters.AddWithValue("@Username", Username);

  15:                 cmd.Parameters.AddWithValue("@password", password);

  16:                 

  17:                 using(SqlDataReader reader = cmd.ExecuteReader())

  18:                 {

  19:                     if (reader.Read())

  20:                     {

  21:                         nUserId = Convert.ToInt32(reader[0]);    

  22:                     }

  23:                 }

  24:             }

  25:         }

  26:         

  27:         if (nUserID > 0)

  28:         {

  29:             // if the user has checked the box

  30:             // store his information so he doesn't have to log in again

  31:             if (cbRememberMe.Checked)

  32:                 FormsAuthentication.SetAuthCookie(nUserID.ToString(), true);

  33:             else

  34:                 FormsAuthentication.SetAuthCookie(nUserID.ToString(), false);

  35:  

  36:             BusinessLogicLayer.User objUser = new User(nUserID);

  37:  

  38:             // store session variables for later reference;

  39:             Session["UserID"] = nUserID;

  40:             Session["Username"] = objUser.Username;

  41:             Session["Password"] = objUser.Password;

  42:             Session["AccessLevel"] = objUser.AccessLevel;

  43:             Session["AccessLevelName"] = Enum.GetName(typeof(Utilities.AccessLevel),objUser.AccessLevel);

  44:  

  45:             Response.Redirect("~/admin/admin.aspx");

  46:         }

  47:         else

  48:         {

  49:             View = Views.Failed;

  50:         }

  51:     }

  52: }

While the code above is functional, it does way too much.  In this button click event there are:

• Call to database explicitly
• logic to set an authorization cookie
• Some session variables being set

Imagine for a moment that you now want to implement a new piece of functionality, user impersonation.  In order to accomplish this with the path of least resistance, which developers commonly take, would be to copy/paste all of the login logic to another UI event somewhere else in the site.  The issue copy/paste development is that any future change to the login logic has to be copied again or else the impersonation feature isn't really impersonating a users experience.  The insidious nature of copy/paste development is that while often it works, it creates a brittle application.

In a recent application we were wanting to add user impersonation.  Below is an example of an implementation of user impersonation.  Adding the feature to the application was gleefully simple.  The ease in adding impersonation was due entirely to the proper granularity in our application components.  Seriously, this is all the code it took:

   1: public ActionResult ImpersonateUser(int id)

   2: {

   3:     var authService = Container.Resolve<AuthService>();

   4:     var userRepository = Container.Resolve<UserRepository>();

   5:  

   6:     var userToImpersonate = userRepository.GetById(id);

   7:     if (userToImpersonate != null)

   8:     {

   9:         authService.SignIn(userToImpersonate);

  10:     }

  11:  

  12:     return RedirectToAction("Index", "Home");

  13: }

I did not know we were going to want user impersonation for this application when we started.  However because we developed the site with certain principles in mind, we were able to reuse pieces of code in ways not originally intended for added behavior.  As you can see from the code above, there are no "clever hacks" or tricks to get impersonation to work.  The beauty of the code above is that if we change the implementation of "SignIn(user)" all appropriate areas will follow the same rules since all areas use the same code.

As I'm closing I want to challenge those of you who have read this far to stop yourself the next time you find yourself copying code from one location to another.  Is there an abstraction or component you're overlooking?  Think about the long term and ask if you're really making the wisest decision for the long term health of your application.  It will be through this introspection and honesty that you'll find areas where you can write smaller component which will allow you to realize the oft spoken benefit in OOP of code reuse.

The following is an email I wrote to the upper management in our company when asked about things we can do to improve the process and what are the next steps of things we need to fix in our application.  It remains mostly unedited and contains my thoughts on what will make our company better when it comes to the software we're writing.  It wasn't written as a blog post.  It wasn't until after I finished with it that I realized that it was decent content for a blog post.  You'll find a very "XP-ish" theme throughout, and that isn't necessarily an accident.  Hope you enjoy! Please leave feedback if something moves you.

Overview

The following contains my thoughts on software development at Super Awesome Company. While I can address the specifics of the project that I see need to be fixed and modified (the “what”), I am instead going to focus on how these items get fixed. I am choosing to do this because over time the new website will have bugs that need to be fixed, enhancements that need to be made, or new features requested. For the long term health of the project it does not matter so much “what” is implemented but rather “how”. For that reason I am avoiding specific project level tasks in this document and speaking at a higher level.

Software has been being developed for over 30 years. While the internet is relatively young, software development by comparison is not. Super Awesome Company will benefit from a commitment to good software development practices. Further, it takes a great deal of commitment and discipline to reap the benefits these practices can bring. Many of my suggestions are based on these principles, which are principles I have committed to learning and adopting over the last several years.

The overarching suggestion I would make is make quality the focus. In order to produce quality, we must slow down. In order to go fast you must go slowly. Sacrificing quality should never be an option. Consider a car manufacturer who needs to get 100 cars built in a single day. If they can only build 80 cars a day, is it better for them in the long run to get 100 cars out the door but have problems or 80 problem free cars? Building deficient cars carries more expense that just fixing the broken cars, such diminished brand reputation. If the goal is 100 cars per day, measures should be sought to increase production while keeping quality high.

We don't need to look too far to see where in a rush we've done something that we've had to revisit several more times because we were "going fast". We've been rushed for nearly a year and I can't say that we are any further along than we would have been had we approached the project more methodically. In some areas we may be further behind where we would have been since we "made sacrifices" to get things done, which have ultimately only put us further behind. Again, to go fast we must go "slow" (methodical). Put another way, quality breeds speed.

Any process, principle, practice we choose should have quality at its heart. Regardless of what process, principles, practices we adopt they will only be successful if we are disciplined enough to follow them.

Planning

Requirements

Requirements need to be clear and understood by all parties. Before developers set out to coding we need to have a clear understanding of what the problem is. The developer is not able to provide a solution until the problem is fully known and understood. Often the perceived solution to a problem is not the best solution. Failure to fully understand the problem/scenario results in rework.

Releases

We need to have planned releases. The timeframe can be discussed and agreed upon by the team. In order to have successful releases we need to plan each release. Each release should be broken down into smaller time frames, iterations. During the iteration the work should be stable for each developer. In other words, they know what they’re going to work on and accomplish (see requirements above) for any given time period.

Metrics

We need to be measuring metrics for the purposes of planning. If we get 10 units of work done each week on average and we have 20 units of work to do, we can easily figure out what is left to do. Without any metrics gathering we can’t do any planning and we’re really flying blind. This will also help with project post mortems. For example, the “web order manager” project was originally estimated to take a week worth of development effort. Three months later, it is still not finished. This is a combination of putting someone with limited knowledge of the business estimating the project, an overly aggressive developer estimate, and having no metric to pull from on how long something should take.

Flexibility

We need to be disciplined in the process of building software, but also realize when the process needs tweaking that we should respond and not be afraid to tweak the process to meet the needs of our team

Design

Simplicity

We should strive for simplicity in our designs. “Clever hacks” are often not healthy for the life time of the project. Simple designs are easier to understand and improve upon. Simple designs also allow other developers ease in working on code. This is much harder to do in practice but it always pays off in the long run.

Design aspects dovetail with testing (later in this document). Typically complex designs prove to be hard to test. When we write code based on simpler designs we’ll find that our code is easier to test. We want both, simple designs and easy-to-test code.

Coding

Standards

We should implement a standard for code development. Right now you can find three different styles/methods of coding in the code base. This makes it hard for developers to work on code they did not author. Projects I work on outside of work require all new code meet the standard, otherwise it is rejected until it meets this standard. As a consumer of these projects I’m always grateful when I can move through any of the project pieces and the code always looks the same.

Testing

Given the environment and complexity of the code, we need to require the developers to be able to test their code in an automated fashion. Visual verification is orders of magnitude slower than automated testing. Once an automated test is written, it runs every time. By having a large collection of tests we can makes changes to certain parts of the application and use existing tests as a safety net to verify nothing else has broken. We currently have over 1,150 tests (about 30% of code has tests) that run whenever someone commits code into source control. Those tests run in about 20 seconds. Imagine how long it would take a user to test each of those cases visually every time a piece of code changes.

Two weeks ago I sat in a meeting adding functionality to shipping and enhancing rules around shipping hazardous items to Canada while people were finding issues because of a solid base of tests. Before the meeting was over both issues discovered were fixed and tests were added for these specific issues so we would be guarded against this happening again.

Unit Tests

All code must have unit tests that can be run in an automated fashion. If the code cannot be unit tested (tested in isolation from other components) it should be restructured in such a way that it can be tested. Very few pieces of code are truly not testable. As such we should have very few areas in our code base which are not tested.

Proving Bug Fixes Through Tests

When bug is found a test should be written before any attempt to fix the bug. This test proves to the developer the bug exists but secondly gives the developer a marker to know when the bug is fixed. As a benefit, this new test acts as a guard against this particular bug ever happening again. This test, when first written, should fail because the developer has not fixed the bug yet. The developer should then work to make the test pass, and by extension fixing the bug. The cost of adding the test is very small when compared against having the test guard against this condition ever happening again.

Code Coverage

We should implement a standard for new code coverage, code that is tested by an automated test. I would recommend somewhere in the 80%-90% range for code coverage. In order to write tests the developers will be forced to think about their code a bit more thoroughly.

Pair Programming

We should adopt pair programming as a method to both increase shared knowledge but also as a means to quality. Pair programming is two developers sitting at one computer for a period of time while code is written. From the outside it may seem impractical to devote two developers to a single computer, thinking it is inefficient. Industry studies however show that code written in a pair has a four-fold benefit:

1. 15% less code– Studies showed that less code is written in order to implement the same functionality. Less code means less to maintain and understand. Less code is also typically simpler.
2. Less bugs – Developers, when working in pairs, can catch each other’s bugs before they ever make it to production. We therefore save time that it would take to fix the bugs that would otherwise go undiscovered.
3. Shared domain knowledge – When “pairing”, each developer is training the other and sharing knowledge and rules around the business. This avoids silos and situations where only one person knows the code.
4. Increased Skill – When “pairing”, design, coding, and testing techniques are transferred between developers. This is often why many development shops pair their senior developers with junior developers; to raise the skill level of the junior developer.

Collective Code Ownership

No one person should be a silo/bottleneck for any area of the code. Any person should be free to make suggestions, fix bugs, or refactor any part of the code. Pair programming (see above) is one tool which seeks to address this.

Conclusion

While the above recommendations may seem like a lot, the above items all really go hand-in-hand. Consider:

• Collective Code Ownership is had through Pair Programming
• Standards are enforced by developers when Pair Programming
• Writing Unit Tests typically leads to Simple Designs
• A commitment to Code Coverage leads to more Unit Tests
• Gathering Metrics leads to better estimates and Release planning

Adopting the above will move us in a positive direction and ensure that all projects coming from our department will be of ever heightening quality. Once the quality bar is set, then we will begin to see the pace pick up; Remember, quality is a prerequisite to speed.

S.O.L.I.D. principles seem to be a hot topic.  The guys over at LosTechies.com explored it last year in March as their topic of the month and did a great job of it. I know when I read blogs I enjoy when different authors post on similar topics.  I find that often the different examples and explanations paint a clearer picture than can be had with only one example.  I'd like to share an area where we use the Single Responsibility Principle (SRP) in our application

Note: The "aha" moment won't be some thrilling display of how we averted some disastrous bug or how we eliminated some thousands of lines of code.  It will be much more subtle but that's ok.  A good design should strive for an element of simplicity.  Great design is asking to hear the answer to a riddle and then thinking, "duh!  of course"  When you hear the answer you realize how "obvious" it was.

Background

In our application we queue all emails to be handled by a different process since sending email is a blocking call which slows your response times (Note to BCL junkies: yes I know there is a SendAsync method).  There are a few instances in our application where we want email to be sent right away and not queued. An example of this would be a confirmation email for a new account or a password reset request.

The Fix

We already had a QueuedEmailService class so adhering to SRP, I built a RealTimeEmailService class whose job, and only job, it is to send the email right away.  Thinking of responsibility driven design, classes:

• Know things
• Do things
• Make decisions

With that in mind I needed another class to make the decision to when to send the email through the RealTimeEmailService or when to use the QueuedEmailService:

(At this point some of you might be reading thinking how easy of a problem this would be to solve and you're quite right.  What I want to point out is not the problem, but how one solves the problem.  Nearly anyone could've gone into the QueuedEmailService and added an if statement and sent emails real-time if that is what is needed, thus ignoring SRP.  By lumping all of the work together you aren't building object oriented software...sorry.  And while you might be okay with that and you're software WILL work, you run the risk of more maintenance issues and modularity problems).

Here's what a quick summary of what I've got:

   1: public QueuedEmailService : IEmailService

   2: {

   3:     public void Send(...)

   4:     {

   5:         // add to some queue

   6:     }

   7: }

   8:  

   9: public RealTimeEmailService : IEmailService

  10: {

  11:     public void Send(...)

  12:     {

  13:         // send right now

  14:     }

  15: }

  16:  

  17: public PriorityBasedEmailService : IEmailService

  18: {

  19:     public void Send(...)

  20:     {

  21:         if (priority == MailPriority.High)

  22:             // send using realtime service

  23:         else

  24:             // send using queued service

  25:     }

  26: }

  27:  

  28:  

I know many people who would look at the code above and think it is possibly overkill.  I would disagree for a few reasons:

• Each class does only one thing.  If we have a problem with some piece of functionality it is much easier to debug.  Each of the three classes above in their full implementation will fit on your screen without the need for scrolling.  It's quite refreshing.
• Modular design - since the application only knows that it's dealing with an IEmailService I can easily swap out implementations.  For example, if we decide that we want all emails to be sent real-time, I just change how I've configured Windsor (my IoC container) and it works, because I have a concrete class which sends real-time.  If you were to combine all of your code into one class, you'd have to go back and change your code and go through the pain of deployment.
• Open-Closed principle - since each of the classes only have only one thing they do, I can more easily say they are closed for modification, reducing the chance that I introduce a bug later on.  If you put this code into one big method, you can change the code to meet your new needs, but each change introduces the possibility of introducing a bug.

There are cases when a design can go overboard, but for the vast majority of cases this simply isn't the case.  I'd rather see a solution where someone has gone too far than what I typically see in classes that do it all, with no clear responsibility.

I hope this helps augment your knowledge of SRP.  If you're still a little fuzzy on SRP and what it is, let me know, I'd love to discuss it with you.