Refactor for Unit Testing

Question

I've been having a little trouble writing Unit Tests for my Business Logic layer, please point me in the right direction. Any advice would be appreciated.

Business Logic

public class TitleLogic
{
    private readonly TitleDAL titleDAL = new TitleDAL();
    private readonly List<TitleEntity> titleEntities;

    public TitleLogic()
    {
        titleEntities = titleDAL.GetAllTitles().ToList();
    }

    public TitleEntity InsertTitle(TitleEntity titleEntity)
    {
        if (!titleEntity.IsValid)
        {
            throw new EntityException<TitleEntity>("Invalid Title.", titleEntity);
        }

        titleEntity.TitleName.TrimSize(TitleEntity.TitleName_Length);

        var createdTitle = titleDAL.InsertTitle(titleEntity);

        titleEntities.Add(createdTitle);

        return createdTitle;
    }

    public TitleEntity FindTitle(string titleName)
    {
        return titleEntities.Find(p => p.TitleName == titleName);
    }
}

Data Layer

public class TitleDAL
{
    private readonly NLog.Logger logger = NLog.LogManager.GetCurrentClassLogger();

    public TitleEntity InsertTitle(TitleEntity titleEntity)
    {
        XTime900Entities xTime900Entities = new XTime900Entities();

        //Find the next CodeId to use
        var titleCodeId = xTime900Entities.TITLEs.Max(p => p.TITLE_CODEID) + 1;

        TITLE title = new TITLE
        {
            TITLE_CODEID = (short) titleCodeId,
            TITLE_ACTIVE = Convert.ToInt16(titleEntity.TitleActive),
            TITLE_NAME = titleEntity.TitleName
        };

        xTime900Entities.TITLEs.InsertOnSubmit(title);
        xTime900Entities.SubmitChanges();
        logger.Debug("Inserted New Title CodeId: {0}", titleCodeId);
        xTime900Entities.Dispose();

        return titleEntity.Clone((short)titleCodeId);
    }

    public ICollection<TitleEntity> GetAllTitles()
    {
        logger.Debug("Retrieving List all Titles from XTime900 database.");
        List<TitleEntity> titleEntities = new List<TitleEntity>();

        using (XTime900Entities XTEntities = new XTime900Entities())
        {
            var titlesInDB = from p in XTEntities.TITLEs
                                  select p;

            foreach (var titlesInDb in titlesInDB)
            {
                TitleEntity genderEntity = new TitleEntity(titlesInDb.TITLE_CODEID)
                {
                    TitleActive = Convert.ToBoolean(titlesInDb.TITLE_ACTIVE),
                    TitleName = titlesInDb.TITLE_NAME
                };

                titleEntities.Add(genderEntity);
            }
        }

        logger.Debug("Found {0} Titles.", titleEntities.Count);
        return titleEntities;
    }
}

Entity

public class TitleEntity
{
    public const int TitleName_Length = 30;

    public short TitleCodeId { get; private set; }
    public bool TitleActive { get; set; }
    public string TitleName { get; set; }
    public bool IsValid
    {
        get
        {
            return !String.IsNullOrEmpty(TitleName);
        }
    }

    public TitleEntity()
    {
        this.TitleCodeId = -1;
    }
    public TitleEntity(short titleCodeId)
    {
        this.TitleCodeId = titleCodeId;
    }

    public TitleEntity Clone(short titleCodeId)
    {
        TitleEntity genderEntity = new TitleEntity(titleCodeId)
        {
            TitleActive = this.TitleActive,
            TitleName = this.TitleName
        };

        return genderEntity;
    }

    public override string ToString()
    {
        StringBuilder sb = new StringBuilder();
        sb.AppendLine(String.Format("TitleCodeId : {0}", TitleCodeId));
        sb.AppendLine(String.Format("TitleActive : {0}", TitleActive));
        sb.AppendLine(String.Format("TitleName : {0}", TitleName));
        return sb.ToString();
    }

    public static bool operator ==(TitleEntity x, TitleEntity y)
    {
        return (x.Equals(y));
    }

    public static bool operator !=(TitleEntity x, TitleEntity y)
    {
        return !(x.Equals(y));
    }

    public bool Equals(TitleEntity other)
    {
        if (ReferenceEquals(null, other)) return false;
        if (ReferenceEquals(this, other)) return true;
        return other.TitleCodeId == TitleCodeId && other.TitleActive.Equals(TitleActive) && Equals(other.TitleName, TitleName);
    }

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        if (ReferenceEquals(this, obj)) return true;
        return obj.GetType() == typeof(TitleEntity) && Equals((TitleEntity)obj);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            var result = TitleCodeId.GetHashCode();
            result = (result * 397) ^ TitleActive.GetHashCode();
            result = (result * 397) ^ (TitleName != null ? TitleName.GetHashCode() : 0);
            return result;
        }
    }
}

Answer 1

You want to test each type in isolation. That is, when you write tests for your business objects you don't want to be forced to exercise the code for the DAL and helper objects that it uses (like XTime900Entities, etc).

Right now, all these types are tightly coupled to eachother, which is unnecessary and is a problem from a testability perspective. That is, a unit test for your business object class is forced to be coupled to your data access layer and implementation. That won't work in the long run. It wont scale over a large code base and unit test maintenance will skyrocket over change.

Also, you need to take exceptions into concern here. For instance, xTime900Entities.Dispose(); will not be called if there is an exception in the middle of that method. This means that your code will be leaking resources if something unexpected happens during InsertTitle. It is a general concept, but something like this would be better in this case:

XTime900Entities xTime900Entities = new XTime900Entities() { // rest of method } // dispose called here automatically, regardless if an exception is thrown in the block or not

Good practices is injecting dependencies, depend on abstractions, to isolate concerns, which allows you to test in isolation.

Answer 2

You can't easily test your business logic as you have created your DAL component inside your TitleLogic class.

First thing I would do is make TitleDAL implement an ITitleDAL interface and make TitleLogic class take an instance of ITitleDAL interface.

Then when you are testing the InsertTitle method you can have tests which:

• check that when an invalid TitleEntity is given the EntityException is thrown
• When the entity is valid that ITitleDAL.InsertTitle is called.
• When a title is correctly inserted, it can be found using the FindTitle method

In your tests you will need to create a mock ITitleDAL implementation (or use a mocking library to create one for you) so that your tests return known expected data are not dependent on the actual DAL.

you might also want to consider testing for:

• What happens if the InsertTitle method on ITitleDAL fails?

Answer 3

First thing you need to do is to think about dependency injection. The simplest way for this to work would be to implement an interface for the DAL, along the lines of

interface ITitleDAL
{
    TitleEntity InsertTitle(TitleEntity titleEntity);
    ICollection<TitleEntity> GetAllTitles();
}

then make your DAL layer implement the interface.

Next, change the constructor for your DAL to accept an object that implements that interface...

public TitleLogic(ITitleDAL myDAL)
{
    titleDAL = myDAL;
    titleEntities = titleDAL.GetAllTitles().ToList();
}

Then create a mock version of your DAL that also implements the interface, but returns static data.

After that, you need to do 2 things.

1) Make your production code create the DAL instance and pass it into the constructor of your business layer.
2) Make your unit test pass in an instance of your mocked class to the constructor, and test against the known data you have coded.