Is parameterized logic always harmful?

I’ve read plenty of times that parameterized logic is considered harmful in testable design.  What’s parameterized logic?  It’s stuff like this:

int Calculate(int lhs, int rhs, Operation op)
{
    if (Operation == Operation.Add)
    {
        return lhs + rhs;
    }
    else if (Operation == Operation.Subtract)
    ...
}

From what I understand, testable design would say this method needs to be refactored.  It does multiple things, and the op parameter dictates what happens.   The reason it’s discouraged is you end up encoding contract for *all* of the operations.  For example, if there is an Operation.Divide then the method will throw an exception if rhs is 0.  Testing this method is possible, but will be tedious.  Documenting this method will be a pain as well.

One alternative would be to replace the single Calculate() method with individual methods like Add(), Subtract(), and so on.  Another technique would involve creating an interface for a type that can operate on two numbers and letting Calculate() take one of those:

int Calculate(int lhs, int rhs, Operator op)
{
    return op.Operate(lhs, rhs);
}

Obviously the injection technique is more complicated, but it provides flexibility: in a library it allows the ability for users to provide their own implementations of Operator that perform operations that might not be built in.  Calculate() still has multiple behaviors, but the testing and documentation burden is shifted to the Operation implementations.

So what got me thinking about this?  Someone asked me how to convert to and from hexadecimal strings in .NET.  I realized there’s three ways to do it: Parse(), TryParse(), and Convert class methods.  I don’t like the thought of convincing someone there’s “one true way” to do things, so I outlined all three and noticed that all three techniques use parameterized logic.

The Parse() and TryParse() techniques will parse a number differently depending on the NumberStyles parameter you pass.  For example, “10” will parse as 10 unless NumberStyles.HexNumber is specified; then it will parse as 16.  Likewise, “B000” is a valid input only when the hex number style is specified.  From a testability standpoint, this seems like it’d require a matrix of tests for just the method.  From a documentation standpoint, Microsoft just documents that a FormatException can be thrown without going into any detail (granted, it’s usually easy to figure out why the exception was thrown.)

I feel like Convert’s methods do things differently.  The relevant methods take the string and an integer that represents the base for conversion.  This still introduces the problem that sometimes 10 means 16 and sometimes “A” is invalid.

So what’s different between the two?  From a user’s standpoint, not much.  If you try to guess the implementation, I bet they’re very different.  Byte.Parse() probably looks like this:

static byte Parse(string input, NumberStyles numberStyles)
{
    if (IsSet(numberStyles, numberStyles.HexNumber))
    {
        // parse hexadecimal
    }
    else
    {
        // parse decimal
    }

    // validate the number using the various other styles
}

Convert.ToByte() could probably look like this:

static byte ToByte(string input, int base)
{
    // some algorithm that parses a number with an arbitrary base
}

I could use Reflector and peek at them, but I’m only using these as examples of the implementation styles I’m interested in (and yes, I know ToByte() only accepts 3 bases.)  I feel like ToByte() here is the lesser of two evils because of the internals.  Parse() has rigid rules that require you to walk through each possible path of the code if you want to create a 100% coverage of the logic.  ToByte() implements an algorithm that should work for many values of base, and thus you can write a smaller number of tests to get full logic coverage.

So I feel like depending on how you implement the parameterized logic, you might reduce the testing and documentation burden.  The Parse() implementation requires a suite of tests for every base and many combinations of the NumberStyles parameter.  The ToByte() implementation requires a suite of tests against an algorithm that takes a parameter.  It feels like that ought to reduce the number of tests; I can imagine using Xunit .NET’s theory tests to dramatically reduce the testing effort.

But I don’t think it’s so straightforward as, “Don’t use parameterized logic unless the parameter is an input to an algorithm.”  Consider what the interface for parsing numeric strings would look like if parameterized logic weren’t available:

structure Int32
{
    // ...

    static Int32 ParseHexNumber(...);
    static Int32 ParseHexNumberAllowWhitespace(...);
    static Int32 ParseHexNumberAllowLeadingWhitespace(...);
    static Int32 ParseHexNumberAllowTrailingWhitespace(...);
    static Int32 ParseHexNumberAllowWhitespace(...);
    ...
}

What a mess! I would not call this an improvement, even if it meant the testing/documentation were more clear.  So it looks like parameterized logic is yet another place where the developer must balance ease of use vs. maintainability.  While the pattern used in my Parse() implementation above is definitely a testing and maintenance nightmare, it is not acceptable to force the user to experience pain.

This entry was posted in .NET and tagged , , . Bookmark the permalink.

Leave a Reply

Your email address will not be published. Required fields are marked *

*

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>