Learning to write DSL utilities for unit tests and am worried about extensibility

Question

I'm trying to simplify our unit tests with hand written DSL's. So far I have DSL's that walk developers through processing a service after setting up all preconditions and the construction of an monster object that has a huge constructor and setters that conflict. The monster is one of the many preconditions of processing the service and they are constructed by hand over and over as developers write unit tests for new services in our system. So it would be worth putting considerable effort into making constructing them easier. I'm tired of watching people copy and paste the "life support system" from unit test to unit test.

I'm making separate new classes, one who's job it is to build the monster class and one that walks them through the steps to process the service. These will be used by many developers. This means once it's used widely changes will be difficult. That's why I'm worried about extendability.

Rather than a simple fluent interface builder (where every method returns a this and is always available) I've decided to use an internal DSL that returns a different nested class. This is a very powerful choice. It lets me limit the methods available at any one step to only the valid ones. It also lets me change what the return type of a method is based on the "state" previous choices put the DSL into.

Briefly, the way it works is the first method returns the first nested class that has the next valid method that could not have been called before. This limits the choices intellisense will offer making writing the DSL's calling code easier.

//Final build() method doesn't exist on PersonBuilder.  It's on a nested class.
public class PersonBuilder {
    public GotRequired doRequired() {
        return new GotRequired();
    }

    //One of many nested classes 
    public class GotRequired {
        public GotFirstName addFirstName(String firstName) {
            mFirstName = firstName;
            return new GotFirstName();
        }
    }

    public class GotFirstName {
        public GotMiddleName addMiddleName(String middleName) {
            mMiddleName = middleName;
            return new GotMiddleName();
        }
    }

    ...

}

This can be used to ensure methods are called in some order and ensure that preconditions have been met. The inner classes can be thought of as states the DSL is in. More than one state change path can be supported. Simply being in a state can tell you what states came before so long as no state merges two state paths into one. This all just means if you're careful you can know every needed method was called before building or returning a result.

Fair warning, what follows is java verbose boiler plate at it's finest. The full listing is here: Monster builder code.

The monsters DSL code looks something like this:

public class PersonBuilder {

// -- Required -- //

private String   mFirstName;
private String   mMiddleName;
...

// -- Required Alternatives -- //  

//Not both; not neither
private int      mBeersToday;     //One or
private String   mHowDrunk;       //the other


// -- What all the fuss is about -- //

private Person   mPerson;

/** Call each required method in order offered.*/
public GotRequired doRequired() {
    return new GotRequired();
}

public class GotRequired {
    public GotFirstName addFirstName(String firstName) {
        mFirstName = firstName;
        return new GotFirstName();
    }
}

public class GotFirstName {
    public GotMiddleName addMiddleName(String middleName) {
        mMiddleName = middleName;
        return new GotMiddleName();
    }
}

...

/*
   Client code example


    .doRequiredAlternatives()   //Either x or y. a, b, or c. etc.
        .addBeersToday(3)       //Now can't call addHowDrunk("Hammered");
        .addFavoriteBeer("Duff")//Now can’t call addJobTitle("Safety Inspector");  
   .addBuild();                 //Calls different constructors based on alternatives

*/
    /** Now the interesting forking bit */
    public class GotRequiredAlternatives {
        public GotPerson addBeersToday(int beersToday){
            mBeersToday = beersToday;

            //Got enough for constructor
            mPerson = new Person(
                    mFirstName,
                    mMiddleName, 
                    mLastName,

                    mNickName,                        
                    mMaidenName, 
                    mEyeColor,   
                    mHairColor,  
                    mDateOfBirth,
                    mBeersToday, //GotBeersToday
                    mAliases);

            return new GotPerson();
        }        

        public GotPerson addHowDrunk(String howDrunk){
            mHowDrunk = howDrunk;

            //Got enough for constructor
            mPerson = new Person(
                    mFirstName,
                    mMiddleName, 
                    mLastName,

                    mNickName,                        
                    mMaidenName, 
                    mEyeColor,   
                    mHairColor,  
                    mDateOfBirth,
                    mHowDrunk, //GotHowDrunk
                    mAliases);

            return new GotPerson();
        }        
    }

    //Could have created GotHowDrunk and GotBeersToday 
    //but we're past the constructor choice so we can 
    //forget the path that brought us here.

    public class GotPerson {
        /** Build Person object leaving optional fields set to default values */
        public Person doBuild() {
            return mPerson;
        }
        /** Call any or none of these optional fields */
        public GotOptional doOptional() {
            return new GotOptional();        
        }
    }

    //Since person is not immutable the only thing gained here is confidence that these were set according to persons whacky rules
    class GotOptional {

        /** Build Person object */
        public Person doBuild() {
            return mPerson;
        }

        public GotOptionalAlternatives doOptionalAlternatives() {
            return new GotOptionalAlternatives();
        }

        public GotOptional addClothing(String clothing) {
            mPerson.setClothing(clothing);
            return this; 
        }

        public GotOptional addTatoo(String tattoo) {
            mPerson.setTattoo(tattoo);
            return this; 
        }

        //Add any number of setters that have good default values and do not conflict with each other

    }

    //Ideally person wouldn't allow this anyway but person is set in stone so at least this provides a safer interface
    public class GotOptionalAlternatives {

        /** Build Person object */
        public Person doBuild() {            
            return mPerson;
        }

        //Optional but conflicting setters. Might never be called.  Must never be called together.


        public GotFavoriteBeer addFavoriteBeer(String favoriteBeer){
            //mFavoriteBeer = favoriteBeer;  //TODO remove
            mPerson.setFavoriteBeer(favoriteBeer); //GotFavoriteBeer

            return new GotFavoriteBeer();
        }

        public GotJobTitle addJobTitle(String jobTitle){
            //mJobTitle = jobTitle;  //TODO remove
            mPerson.setJobTitle(jobTitle); //GotJobTitle

            return new GotJobTitle();
        }
    }

    //These are not strictly needed.  They are like one-statement {}'s after an 'if'.  Simply there if more gets added.

    public class GotFavoriteBeer {
        /** Build Person object */
        public Person doBuild() {            
            return mPerson;
        }
    }

    public class GotJobTitle {
        /** Build Person object */
        public Person doBuild() {            
            return mPerson;
        }
    }
}

I'll spare you the service code that uses the same nested class technique to mix optional methods and required methods. It is also able to return different result types based on previous alternate method choices since even though the result() method has the same name and parameters, it's coming off a different inner class.

Despite the unreal amount of typing required to create these DSLs I've found the results to be very powerful. However, I expect people will want new features added, new preconditions for processing the service. If I use this style is there any hope of reuse or would it be better to just write a whole new DSL each time something needs to change?

Cause once it's working and being used I do NOT want to dive back into the existing DSL classes and mess with them.

If you can see other issues please let me know. I'm hoping for a peer review but I appreciate any feedback.

Answer 1

A few thoughts...

1. It's not at all obvious how the user gets started. new PersonBuilder().blahblahblah()?

   A common alternative idiom is to use a public static method as the bootstrap point. PersonBuilder.newInstance(), PersonBuilder.create(), or even PersonBuilder.start()

2. I'm bothered by the presence of new in the middle of the test setup

   public class GotRequired {
       public GotFirstName addFirstName(String firstName) {
           mFirstName = firstName;
           return new GotFirstName();
       }
   }
   

   addFirstName includes both logic and object construction, which in production logic would be a code-smell. It usually means one of two things; either that you should be injecting a factory to worry about the object construction, or that you should be creating the object earlier.

   In this case, I think creating the object in advance is called for:

   public class GotRequired {
       private final GotFirstName next;
   
       GotRequired(GotFirstName next) {
           this.next = next;
       }
   
       public GotFirstName addFirstName(String firstName) {
           mFirstName = firstName;
           return next;
       }
   }
   

   Presumably, you are doing that because you wanted to share the member objects in the PersonBuilder. Instead, make the record that everybody is editing explicit.

3. Your design has the state transitions scattered all over -- instead of being able to see the entire design in one place, you have to look at each class in order to discover what's going on.

   If you build out the steps in advance, then the "create" method becomes the documentation for how the state machine works.

   The straight forward approach would be to build your objects in reverse order

   ...
   GotFirstName firstNameStep = new GotFirstName(previousStep);
   GotRequired gotRequiredStep = new GotRequired(firstNameStep);
   return new PersonBuilder(gotRequiredStep);
   

   To get a more natural ordering, you could use pass through setters

   public class GotRequired {
       ...
       GotFirstName setNext(GotFirstName next);
           this.next = next;
           return next;
       }
    }
   

4. The past tense names of the steps are really hard to read. The class names should be telling you what this class does, rather than telling you what some previous class did. GetMiddleName rather than GotFirstName, for instance. Get/Got are verbs though, which doesn't likely match your coding standards. MiddleNameStep? MiddleNameCollector?

5. One nasty side effect of the way you've written these classes is that you can't re-use them for a different step ordering. For instance, what if you had another kind of test where middle name should be irrelevant? In that case, GetFirstName should proceed immediately to GetLastName.

   You might be able to patch this by using generics. Each class in turn is specialized on its next step:

   public class GetFirstName<T> {
       private final T next;
   
       GetFirstName(T next) {
           this.next = next;
       }
   
       public T addFirstName(String firstName) {
           mFirstName = firstName;
           return next;
       }
   }
   
   ...
   GetFirstName<GetLastName> thisStep = new GetFirstName(thatStep);
   

6. I'm not convinced that this is the right approach -- simplifying writing tests by constraining the programmers to do the right thing isn't as likely to be successful as actually making the tests simpler. You should look at the source for the tests, and examine how many parameters the developers need to set that aren't actually part of the text. If the developers are specifying 7 or 8 parameters for a test that really only cares about one of them, then you are making extra work.

   Hard to know for sure without an actual example of a test.