Code Design / Testability How To?

Question

My team is designing a library that is wrapping calls to Active Directory to search and return a list of people.

We have a person class that wraps up the information for a person found. We then use the List to wrap them up. When we call the search it uses the internal System.Directory library and returns a SearchResultCollection object. We then iterate through it to build up the list<> and return that.

We have designed the person class to only have read only (get) properties since we don't want the callee to change the person info. We pass in the SearchResult object from the System.Directory library on the constructor of the person.

My issue is we can't test this easily.

My thoughts so far have been:

1. Pass variables into the person constructor for each property needing to be set.

   Unfortunately, this will make for a very long constructor parameter list.... Smells bad to me.

2. Allow the person class to have setters on the properties.

   Again, smells bad to me since we can't control the callee from using this.

3. Refactor:

   I have looked at the extract to interface and adapt parameter techniques. It seems the adapt parameter has the most promise? Adapt parameter seems nice because it does help break the dependency I have on the Directory Library's SearchResult object. So if in the future I wanted to do some other kind of search we are in good shape. At least I think we are?

4. Sub class the person object and create a test Person with setters....

   Seems like it would work but not sure if it's the right way to go?

5. Mock it

   Haven't done any mocking yet so again not sure on this one.

EDIT: If mocking is best idea please let me know... However, I would be interested to know how this would be done without mocking also (or perhaps it really isn't do able without mocking)....

I would appreciate guidance on this one.

Here's a snippet of the code:

    public class PeopleSearcher
{
   .... declarations left out....

    public List<Person> FindPerson(string FirstName, string LastName, string Login)
    {
         ...filter setup left out for brevity....

         _peopleFound = _directoryToSearch.FindAll();
        //Convert to list of persons....
            int cnt = 0;
            _listOfPeople = new List<Person>();
            while (cnt < _peopleFound.Count)
            {
                Person p = new Person(_peopleFound[0]);
                _listOfPeople.Add(p);
                cnt++;
            }
            return _listOfPeople;
        }

    }

    public class Person
    {
        private string sn;
        ....further declarations left out for brevity....

        public Person(SearchResult PersonFound)
        {
            sn = PersonFound.Properties["sn"].Count == 0 ? string.Empty : PersonFound.Properties["sn"][0].ToString();
            givenName = PersonFound.Properties["givenName"].Count == 0 ? string.Empty : PersonFound.Properties["givenName"][0].ToString();
            sAMAccountName = PersonFound.Properties["sAMAccountName"].Count == 0 ? string.Empty : PersonFound.Properties["sAMAccountName"][0].ToString();
            adsPath = PersonFound.Path == null ? string.Empty : PersonFound.Path;

        }

        public string LastName
        {
            get
            {
                return sn;
            }
        }

        .... more getters...
     }
}

Answer 1

In your mock (by framework or otherwise), you're still going to end up having to create Person objects with values, which leads you right back to your original problem.

Fortunately, there are two excellent solutions:

1) Go ahead and add setters to the Person class, but make them protected. This means your mock and test code would have to be in the same package, but would block other users from mutating your Persons. (and we don't want mutants running around - there's been enough of that in the movies lately).

2) Use a Builder class (as Joshua Bloch describes in Effective Java). You'd create a public static PersonBuilder class inside Person, which would export a build method and chainable parameter specifiers (like setters, but not separately callable):

public class Person ....
   public static class PersonBuilder {
      public PersonBuilder (String firstName, String lastName) {...} // my sample has two required values
      public Person build() { ... }
      public PersonBuilder ssn (String value) { ... }
      public PersonBuilder adsPath (String value) { ... }
      ...
   }
   ...
}

The chainable value specifiers look like this:

      public PersonBuilder ssn (String value) { 
         this.sn = value;
         return this;
      }

Then a call to create a Person looks like this:

   Person thisPerson = new Person.PersonBuilder ("John", "Smith").ssn("123-45-6789").adsPath("whatever");

This method completely hides the methods which can set the values (indeed, you have no "setters"), but keeps you from having to deal with long constructor argument lists (which makes it easier to deal with optional values).

Incidentally, you also probably want to make Person's constructor private.

Answer 2

"Mocking" is a word that is usually used for all kinds of test doubles. And most times people or not "mocking", they're faking or stubbing. Anyway, your 4th option (subclass and add setters) sounds to me like the easiest way to go given your codebase assuming you want Person objects to pass toother methods. Because I don't think you're talking about testing that the person object gets the properties set correct by the constructor, right?

Answer 3

Mock it. This is the sort of situation that mocking was invented for. I've only done mocking in Ruby, so I'm not sure of the state of the art for .net, but it should work well.

When mocking it you might realize some areas that should be refactored. This is also a good plan.