Decomplexing: One Dummy’s Strategy for Understanding Hard Code

As you may have guessed, I am working with FitNesse. In particular, I’m making pretty heavy use of FitLibrary, which is a real pain in the keester at times because it is the product of a single person’s efforts, and that person writes code that is mind-numbingly complex. I’m still not sure if I believe that the code needs to be this complex, but I know for sure that from where I’m looking it’s Hard Code. Which is funny in a karma’s-a-bitch kind of way, because I have had at least three guys tell me they find my code very pretty but also pretty hard to understand sometimes.

Anyway. I want to share my strategy for understanding code when you don’t really have the technical expertise to analyze it in situ. Basically here’s what I do: You know how all those books tell you to use tests to guide your development, which will automatically force you to break all of your functionality down into nice, modular, reusable units? Yeah, throw that away, and treat your call stack as a single giant function, taking out all of the bits you don’t care about.

Allow me to demonstrate.

In my case I hit the wall when I tried to figure out how to register a new parser for the double type in CombinationFixture. It took me a while to get my head around what Rick Mugridge did here. He took the FitNesse Fixture hierarchy and just sort of threw that away. FitLibrary’s Fixture classes are all wrappers around Traverse and its various subclasses. Easy enough (once you understand it and don’t, for example, think that a Traverse is a way of “traversing” a test table within a fixture). But then I found this:

[CombinationTraverse]
bindFirstRowToTarget(Row row, TestResults testResults) {
    /* ignored */
    methodTarget = PlugBoard.lookupTarget.findTheMethodMapped("combine", 2, this);
    Parser[] parameterParsers = methodTarget.getParameterParsers();
    /* ignored */
}

You might be wondering why in the world I find that difficult. It’s a simple enough method call, after all. Obviously the public, static member of a class called PlugBoard (in a subpackage called global, no less) could be considered bad design by some, but we’ve all done it now and then. Two hardcoded parameters also looks kind of odd, but again, sometimes you gotta do it. If you dig into PlugBoard’s definition, lookupTarget’s type is LookupMethodTarget, an interface, but happily PlugBoard has a default that sets it to a LookupMethodTargetStandard, so in this particular case it’s not a nightmare of reverse dependency tracking.

No, the thing there that’s really subtle and difficult is the passing in of this. Semantically it makes sense; you’re looking for a method named combine with 2 arguments on the current object. I would expect something like this to live in a static utility class in most cases, but I can respect the requirement for a service locator when you have a framework that aims for universality. If that was all that was happening, I’d have been fine.

Let’s dive into findTheMethodMapped and see what’s going on:

[LookupMethodTargetStandard]
findTheMethodMapped(String name,int argCount,Evaluator evaluator) throws Exception {
    return 
        findMethodOrGetter(evaluator.getRuntimeContext().extendedCamel(name), unknownParameterNames(argCount),"Type",evaluator);
}

Remember, Evaluator is actually the Traverse that we started with. Also, we have a runtimeContext that we may want to keep in mind. Happily it turns out unknownParameterNames is just getting placeholder strings. Stepping down the call stack a little further…

[LookupMethodTargetStandard]
findMethodOrGetter(String name, List methodArgs, String returnType, Evaluator evaluator) throws Exception {
    int argCount = methodArgs.size();
    IScope scope = evaluator.getScope();
    for (TypedObject typedObject : scope.objectsForLookup()) {
        Option target = typedObject.new_findSpecificMethod(name,argCount,evaluator);
        /* ignored */
    }
}

Now we’re really getting somewhere! Wait…What’s the IScope object? How do I backtrack to find out where that was set? Never mind, I’ll just figure out where the objectsForLookup method is defined and go from there (I am at the time of this writing still not sure if ScopeStack is the actual class I should be looking at for this). What’s that Option<> generic doing (turns out it’s syntactic sugar for checking nulls). What’s a TypedObject? Why am I still getting wrappers this far down the stack? Fine, let me dive in again…

[GenericTypedObject]
new_findSpecificMethod(String name, int argCount, Evaluator evaluator){
    Option methodClosureOption = new_findMethodClosure(name,argCount);
    if (methodClosureOption.isSome())
        return new
            Some(methodTargetFactory.createCalledMethodTarget(methodClosureOption.get(), evaluator));
}

Quick note on that call to createCalledMethodTarget – the factory is actually defined as an anonymous class in the GenericTypedObject constructor. More tricky context to hold in our heads! Whee!

[GenericTypedObject]
new_findMethodClosure(String name, int argCount) {
    Closure methodClosure = lookupClosure.findMethodClosure(this,name,argCount);
}

lookupClosure is populated in the GenericTypedObject constructor from PlugBoard.lookupClosure. Universality strikes again – making the GTO constructor overriddeable and injectable means that in some cases you need to override the PlugBoard only for a specific class, I guess, so using the service locator in-line wasn’t an option(?) , adding another element of context to the stack. Checking PlugBoard.java, I see that the default implementation class for this interface is LookupClosureStandard. Further up and further in!

[LookupClosureStandard]
findMethodClosure(TypedObject typedObject, String methodName, int argCount) {
    /* ignored */
    Method chosenMethod =
        findMethod(typedObject.classType(),methodName, argCount, typedObject.getSubject());
    /* ignored */
    return new MethodClosure(typedObject,chosenMethod);
}

[LookupClosureStandard]
findMethod(Class<?> type, String name, int argCount, Object subject) {
    /* ignored */
    Method chosenMethod = findSpecificMethod(type, name, argCount, subject);
    /* ignored */
    return chosenMethod;
}

[LookupClosureStandard]
findSpecificMethod(Class<?> type, String name, int argCount, Object subject) {
    Method[] methods = type.getMethods();
    for (int i = 0; i < methods.length; i++) {
        /* ignored */
        if (chosenMethod == null)
            chosenMethod = method;
        /* ignored */
    }
    return chosenMethod;
}

Hey, a built-in type! Maybe we’re close to the end of this labyrinth! Let’s jump back up the stack!

[GenericTypedObject]
new_findSpecificMethod(String name, int argCount, Evaluator evaluator){
    Option methodClosureOption = new_findMethodClosure(name,argCount);
    if (methodClosureOption.isSome())
        return new
            Some(methodTargetFactory.createCalledMethodTarget(
                methodClosureOption.get(), evaluator));
}

[Anonymous factory type]
createCalledMethodTarget(Closure closure, Evaluator evaluator) {
    return new CalledMethodTarget(closure,evaluator);
};

[CalledMethodTarget]
CalledMethodTarget(Closure closure, Evaluator evaluator) {
    /* ignored */
    parameterParsers = closure.parameterParsers(evaluator);
    /* ignored */
}

Never mind.

[MethodClosure]
parameterParsers(EvaluatorEvaluator){
    return typedObject.parameterParsers(evaluator,method);
}

[GenericTypedObject]
parameterParsers(Evaluator evaluator, Method method) {
    for (int i = 0; i < types.length; i++) {
        Typed parameterTyped = parameterTyped(method, i);
        parameterParsers[i] = getTyped().on(evaluator, parameterTyped,false);
    }
}

[GenericTypedObject]
parameterTyped(Method method, int parameterNo){
    Type givenType = method.getGenericParameterTypes()[parameterNo];
    Type genericParameterType = typed.bind(givenType,describe(method));
    return new GenericTyped(genericParameterType,true);
}

[GenericTyped]
GenericTyped(Type type, boolean hasMethodOrField) {
    this(type);
    this.hasMethodOrField = hasMethodOrField;
}

[GenericTyped]
GenericTyped(Type type) {
    this.type = type;
    /* Thankfully not a generic type so we can ignore the rest */
}

Now we have a working wrapper for our parameter type, so we can go get the parser! Jumping back up to the last place we actually did something with a return value:

[GenericTypedObject]
parameterParsers(Evaluator evaluator, Method method) {
    for (int i = 0; i < types.length; i++) {
        Typed parameterTyped = parameterTyped(method, i);
        parameterParsers[i] = getTyped().on(evaluator, parameterTyped,false);
    }
}

[GenericTyped]
on(Evaluator evaluator, Typed typed, boolean isResult) {
    return parserSelector.parserFor(evaluator, typed, isResult);
}

parserSelector is defaulted to a ParserSelectorForType object.

[ParserSelectorForType]
parserFor(Evaluator evaluator, Typed typed, boolean isResult) {
    Class<?> classType = typed.asClass();
    DelegateParser delegate = ParseDelegation.getDelegate(classType);
    if (delegate != null)
        return delegate.parser(evaluator, typed);
    DomainObjectParser domainObjectParser = new DomainObjectParser(evaluator,typed);
    if (domainObjectParser.hasFinderMethod())
        return domainObjectParser;
    ParserFactory parserFactory = mapClassToParserFactory.get(classType);
    if (parserFactory == null) {
        parserFactory = parserFactory(typed, isResult);
    if (parserFactory != null)
        mapClassToParserFactory.put(classType, parserFactory);
    else {
        if (classType != String.class && classType != Object.class)
            return domainObjectParser;
        return new DelegatingParser(new FailingDelegateParser(typed.asClass()),evaluator,typed);
        }
    return parserFactory.parser(evaluator, typed);
}

I’m not ignoring anything there because it’s all functional. But that’s a whole lot of service location options. How do I find out which one applies to me? I can eliminate ParseDelegation right off the bat; that was the first thing I looked at when I tried to register a parse delegate in the first place. DomainObjectParser is a new one, so I’ll start there. The key there seems to be hasFinderMethod

[DomainObjectParser]
hasFinderMethod() {
    return finder.hasFinderMethod();
}

I need to know about the finder member

[DomainObjectParser]
DomainObjectParser(Evaluator evaluator, Typed typed) {
    /* ignored */
    finder = typed.getFinder(evaluator);
}

From previous work we know typed is a GenericTyped.

[GenericTyped]
getFinder(Evaluator evaluator) {
    return new GenericFinder(this,evaluator);
}

[GenericFinder]
public GenericFinder(GenericTyped typed, Evaluator evaluator) {
    this.typed = typed;
    String shortClassName = typed.simpleClassName();
    referenceParser = EntityReference.create(shortClassName.toLowerCase());
    /* ignored */
    LookupMethodTarget lookupTarget = PlugBoard.lookupTarget;
    List<Class<?>> potentialClasses = lookupTarget.possibleClasses(evaluator.getScope());
    /* ignored */
    findExceptionMessage += ", possibly in classes:"+names(potentialClasses)+"";
    /* ignored */
}

Before I go on, I want to note that that call to LookupTarget.possibleClasses, which at first glance looks like an interesting place to start, is actually there solely for exception reporting purposes. One part of my brain is now screaming at another part to stop this insanity, but I must finish this.

[EntityReference]
EntityReference create(String name) {
    EntityReference ref = entityReferenceCache.get(name);
    if (ref == null) {
        ref = new EntityReference(name);
        entityReferenceCache.put(name,ref);
    }
    return ref;
}

[EntityReference]
EntityReference(String entityClassName) {
    this.entityClassName = entityClassName;
    createMap();
}

[EntityReference]
createMap() {
    for (int form = 0; form < forms.length; form++) {
        final String theForm = forms[form];
        put(theForm,new Integer(0)); // "the"
        put(theForm+" "+entityClassName,new Integer(0)); // "the client"
        put(theForm+" last",new Integer(-1)); // "the last"
        put(theForm+" last "+entityClassName,Integer.valueOf(-1)); // "the last client"
        for (int count = 0; count < counts.length; count++) {
            final String theCountString = counts[count];
            final Integer theCount = Integer.valueOf(count);
            put(theCountString,theCount); // "first"
            put(theForm+" "+theCountString,theCount); // "the first"
            put(theForm+" "+theCountString+" "+entityClassName,theCount); // "the first client"
            put(entityClassName+"#"+(count+1),theCount); // "client#0"
        }
    }
}

[EntityReference]
put(String referenceString, Integer index) {
    mapStringToInt.put(referenceString.toLowerCase(),index);
}

I have no idea what all that does, but it’s obviously not what I’m looking for, so let’s go back and see what else GenericFinder might be doing:

[GenericFinder]
GenericFinder(GenericTyped typed, Evaluator evaluator) {
    /* ignored */
    final Class<?>[] intArg = { int.class };
    final Class<?>[] stringArg = { String.class };
    final Class<?>[] showArg = { typed.asClass() };
    final String findName = evaluator.getRuntimeContext().extendedCamel(FIND+" "+shortClassName);

    /* ignored */

    findIntMethod = lookupTarget.findFixturingMethod(evaluator, findName, intArg);
    findStringMethod = lookupTarget.findFixturingMethod(evaluator, findName, stringArg);
    showMethod = lookupTarget.findFixturingMethod(evaluator, showMethodName, showArg);
    if (typed.isGeneric()) {
        final Class<?>[] genericStringArg = { String.class, Type.class };
        final Class<?>[] genericShowArg = { typed.asClass(), Type.class };
        genericFindStringMethod = lookupTarget.findFixturingMethod(evaluator, findName, genericStringArg);
        genericShowMethod = lookupTarget.findFixturingMethod(evaluator, showMethodName, genericShowArg);
    }
}

A lot of ways this could go, but at least the gateway is the same for all of them.

[LookupMethodTargetStandard]
findFixturingMethod(Evaluator evaluator, String name, Class<?>[] argTypes) {
    IScope scope = evaluator.getScope();
    for (TypedObject typedObject : scope.objectsForLookup()) {
        Closure target = typedObject.findPublicMethodClosureForTypedObject(name,argTypes);
    if (target != null)
        return target;
    }
    return null;
}

[GenericTypedObject]
findPublicMethodClosureForTypedObject(String name, Class<?>[] argTypes) {
    return PlugBoard.lookupClosure.findPublicMethodClosure(this, name, argTypes);
}

I want to note at this point that we’ve handed this Evaluator around and we are STILL not actually using most of the object itself. Instead we’re going back to the Scope object and thence to the PlugBoard, which are populated out in the world somewhere but have almost no relationship to what’s going on in context. The level of isolation on display is wanting at best.

[LookupClosureStandard]
findPublicMethodClosure(TypedObject typedObject, String name, Class<?>[] argTypes) {
    if (typedObject.isNull())
        return null;
    try {
        return new  MethodClosure(typedObject,typedObject.classType().getMethod(name,argTypes));
    } catch (Exception e) {
        return null;
    }
}

It seems like this isn’t going to work, because double does not have a method whose name starts with find. We have to jump right back up the stack to

[ParserSelectorForType]
parserFor(Evaluator evaluator, Typed typed, boolean isResult) {
    /* ignored */
    parserFactory = parserFactory(typed, isResult);
    /* ignored */
    return parserFactory.parser(evaluator, typed);
}

[ParseSelectorForType]
private ParserFactory parserFactory(Typed typed, boolean isResult) {
    // The order of the applicability tests here is significant
    Class<?> classType = typed.asClass();
    if (Map.class.isAssignableFrom(classType))
        return mapParserFactory();
    if (SetParser.applicableType(classType))
        return setParserFactory();
    if (ArrayParser.applicableType(classType))
        return ArrayParser.parserFactory();
    if (ListParser.applicableType(classType))
        return listParserFactory();
    if (TreeParser.applicableType(classType))
        return TreeParser.parserFactory();
    if (TableParser.applicableType(classType))
        return TableParser.parserFactory();
    if (GraphicParser.applicableType(classType))
        return GraphicParser.parserFactory();
    if (TaggedStringParser.applicableType(classType))
        return TaggedStringParser.parserFactory();
    if (classType == Tables.class)
        return TablesParser.parserFactory();

    ParserFactory factory = LookupPropertyEditorBasedParser.parserFactory(typed);
    if (factory != null)
        return factory;
    factory = ParseDelegation.selfParseFactory(typed);
    if (factory != null)
        return factory;
    if (typed.isEnum())
        return EnumParser.parserFactory();
    if (canTreatAsString(classType, typed.hasMethodOrField(), isResult))
        return ByStringParser.parserFactory();
    return null;
}

It’s kind of ugly, but at least it’s just ugly. Ugly I can handle. Going down through the list, I’m thinking the only likely options here are TaggedStringParser, LookupPropertyEditorBasedParser, ParseDelegation, or ByStringParser. After a quick search, it seems like maybe the selfParseFactory might do the trick.

[ParseDelegation]
selfParseFactory(final Typed typed) {
    final DelegateParser classParser = findSelfParser(typed);
    if (classParser != null)
    return new ParserFactory() {
        public Parser parser(Evaluator evaluator, Typed typed2) {
            return new DelegatingParser(classParser,evaluator,typed2);
        }
    };
    return null;
}

[ParseDelegation]
findSelfParser(final Typed typed) {
    DelegateParser classParser = SelfParser.findSelfParser(typed.asClass());
    if (classParser == null)
        classParser = SelfConstructorParser.findSelfConstructorParser(typed.asClass());
    return classParser;
}

[SelfParser]
findSelfParser(Class<?> type) {
    Method parseMethod = findParseMethod(type);
    /* ignored */
}

[SelfParser]
findParseMethod(Class<?> type) {
    try {
        Method method = type.getMethod("parse", new Class[]{ String.class });
        /* ignored */
        return method;
    } catch (NoSuchMethodException e) {}
    return null;
}

Ok, so we have a winner. Jump back up a bit…

[SelfParser]
findSelfParser(Class<?> type) {
/* ignored */
return new SelfParser(parseMethod);
}

[SelfParser]
public SelfParser(Method parseMethod) {
this.parseMethod = parseMethod;
}

At this point I jumped back up to see what I should do with this only to realize that I was in the wrong call stack altogether. The site where we actually check the value is CombinationTraverse.processRow:

[CombinationTraverse]
processRow(Row row, TestResults testResults) {
    /* ignored */
    for (int i = 1; i < row.size(); i++) {
        Object result = methodTarget.invoke(new Object[]{arg1,topValues.get(i-1)});
        methodTarget.checkResult(row.at(i),result,true,false, testResults);
    }
    /* ignored */
}

[CalledMethodTarget]
checkResult(Cell expectedCell, Object initialResult, boolean showWrongs,
boolean handleSubtype, TestResults testResults) {
    /* ignored */
    ResultParser valueParser = resultParser;
    /* ignored */
    if (valueParser.matches(expectedCell,result,testResults)) {
        expectedCell.passIfNotEmbedded(testResults,runtime);
        /* ignored */
    }
}

A quick check to find resultParser’s initialization finds this

[CalledMethodTarget]
CalledMethodTarget(Closure closure, Evaluator evaluator) {
    /* ignored */
    resultParser = closure.resultParser(evaluator);
}

[MethodClosure]
public ResultParser resultParser(Evaluator evaluator) {
    return typedObject.resultParser(evaluator,method);
}

[GenericTypedObject]
public ResultParser resultParser(Evaluator evaluator, Method method) {
    Typed resultTyped = resultTyped(method);
    return new GetterParser(getTyped().on(evaluator, resultTyped, true),method);
}

[GenericTypedObject]
protected Typed resultTyped(Method method) {
    Type genericReturnType = typed.bind(method.getGenericReturnType(),describe(method));
    return new GenericTyped(genericReturnType,true);
}

We’ve already traversed the call stack for on(), so the wrong-stack effort wasn’t entirely wasted. GetterParser turns out to be extremely simple:

[GetterParser]
GetterParser(Parser parser, Method method) {
    this.parser = parser;
    this.method = method;
}

The problem I have at this point is simple – everything I can see here indicates that I should be able to register my ParseDelegate for double.class and it should work exactly as I expect it to. And that just does not happen.

But at least I know more or less what is going on.