Testable MVC Models - Part 1: The Naive Approach

by Larry Spencer Wednesday, April 17, 2013 4:53 AM

You've made the switch from old-style ASP.NET Web Forms to ASP.NET MVC or some other flavor of M-V-Whatever. You have separated your user interface and your database from your business logic and life feels pretty good.

I'm here to tell you that it could be even better. In your euphoria over having real Models at all, you may be experiencing frustrations that you don't even know you have.

Take this little beauty as an example. It encapsulates the simplest requirements I could think of:

  • Contact consists of a name and an email address.
  • The Contact is valid if both Name and Email are supplied, and the email address is valid according to a regular expression that I stole from Stack Overflow.
  • The Contact cannot be persisted (the Save method) unless it is valid.

 

using System;
using System.Text.RegularExpressions;
using Interfaces;

namespace BusinessLogicLayer
{
    public class Contact1
    {
        // The repository injected from the constructor.
        private IRepository<Contact1, int> Repository { get; set; }

        /// <summary>
        /// The object's key in the repository, or 0 if the object has not 
        /// yet been persisted.
        /// </summary>
        public int ContactId { get; set; }

        /// <summary>
        /// Get/set the contact's name.
        /// </summary>
        public string Name { get; set; }

        /// <summary>
        /// Get/set the contact's email address.
        /// </summary>
        public string Email { get; set; }

        /// <summary>
        /// Tell whether the object is in a valid (saveable) state.
        /// </summary>
        public bool IsValid { get { return GetError(false)==null; } }
        
        /// <summary>
        /// Constructor.
        /// </summary>
        /// <param name="repository">An object that can persist the contact.</param>
        public Contact1(IRepository<Contact1, int> repository)
        {
            Repository = repository;
        }

        /// <summary>
        /// Persist the object, but only if it's valid.
        /// </summary>
        /// <returns>The object's ContactId. If saving the object for the first
        /// time, this will be the newly assigned key.</returns>
        /// <exception cref="System.InvalidOperationException">The object is not
        /// in a valid state.</exception>
        public int Save()
        {
            GetError(); // Throws if there's an error.
            return ContactId=Repository.Save(this);
        }

        /// <summary>
        /// Validate this object.
        /// </summary>
        /// <param name="exceptionOnError">If true, and the object is invalid,
        /// an Exception will be thrown.</param>
        /// <returns>An error message, or null if there is no error.</returns>
        public string GetError(bool exceptionOnError = true)
        {
            Exception exception = null;
            if (String.IsNullOrWhiteSpace(Name))
                exception = new InvalidOperationException(Properties.Resources.NameIsRequired);

            else if (String.IsNullOrWhiteSpace(Email))
                exception = new InvalidOperationException(Properties.Resources.EmailIsRequired);

            // Thanks to http://stackoverflow.com/questions/5342375/c-sharp-regex-email-validation for the Regex!
            else if (!Regex.IsMatch(Email,
                           @"^([0-9a-zA-Z]" + //Start with a digit or letter
                           @"([\+\-_\.][0-9a-zA-Z]+)*" + // No contiguous or ending +-_. chars 
                           @")+" +
                           @"@(([0-9a-zA-Z][-\w]*[0-9a-zA-Z]*\.)+[a-zA-Z0-9]{2,17})$"))
                exception = new FormatException(Properties.Resources.EmailInvalid);

            if (exception == null)
                return null;
            if (exceptionOnError)
                throw exception;
            return exception.Message;
        }
    }
}

 

Not only does the code meet the requirements, but I've even injected an interface for the repository so the model is decoupled from the database. (I have skipped over some details for clarity, though. There are no Code Contracts, the property setters have no validation, and so forth.)

So what do we want to test? 

  • The IsValid property should return true if both Name and Email are non-blank, and Email is valid. Otherwise, it should return false.
  • The GetError method should return null if there is no error (same conditions as above). If there is an error, it should either throw it or return it, according to the second parameter.
  • The Save method should throw an error if the object is not valid. Otherwise, it should write to the repository.

You may be starting to see the problems already.

First, our tests of all three members (IsValid, GetError and Save) really ought to involve testing all the possible conditions: Name missing, Email missing, Email present but invalid, and everything OK. Sure, we can inspect the code and see that Save is calling GetError and, on that basis, write only one unit test for attempting to save an invalid object. But that would be cheating. And how do you know someone will not change the code someday so IsValid, GetError and Save are no longer consistent? When the code is as simple as my toy example, we might be confident that will never happen, but we all know that real life is not that nice.

Second, did you notice how I blithely used the phrase "Email is valid"? Email addresses are surprisingly complicated. There should be more tests of what it means to be valid or invalid than all the other tests put together. But because my Conctact1 class is not primarily about email addresses, I unconsciously forgot to be thorough in that area.

There is a third consequence, and it derives from the second. The test class will have to know what it means for an email address to be valid. Now both the concept of an email address, and the tests of it, are in the wrong place. All the work we go to will be completely non-reusable. Other classes that have email addresses will have to start from scratch.

The root of all these problems is that Contact1 violates the Single Responsibility Principle. Consider how many responsibilities it has:

  1. Grouping things together as a model.
  2. Determining whether the model is valid.
  3. Knowing what constitutes a valid email address. This is a violation of the Law of Demeter -- that little-understood principle of software engineering that says an object should only talk to its immediate neighbors. It's also known as the Law of Only One Dot. Although Email is not an object, what our unit tests must do is tantamount to Contact1.Email.IsValid -- two dots.

Those may seem like related concerns, and from a business perspective they are, but we have already seen the difficulties they present for unit testing. In the downloadable solution (SrpAndTestability.zip (198.97 kb)), you can see the duplication and incompleteness in Contact1_UnitTests.cs.

In the next post, we will see a better approach.

Tags: , , ,

All | ASP.NET MVC | Dependency Injection | Talks

Pingbacks and trackbacks (1)+

Add comment

About the Author

Larry Spencer

Larry Spencer develops software with the Microsoft .NET Framework for ScerIS, a document-management company in Sudbury, MA.