Problematic parameters

Parameterization can be a very good thing. It is an oft-employed antidote to unfortunate hard-coded values:

// BAD... we can't specify the file name!
public async Task SaveAsync()
{
    using (FileStream stream = this.GetAsyncStream("OutputFile.txt"))
    {
        byte[] buffer = this.Deserialize();
        await stream.WriteAsync(buffer, buffer.Length);
    }
}

// Good! Now I can save to a different folder.
public async Task SaveAsync(string path)
{
    using (FileStream stream = this.GetAsyncStream(path))
    {
        byte[] buffer = this.Deserialize();
        await stream.WriteAsync(buffer, buffer.Length);
    }
}

Generic type parameters can transform truly type-independent code without boxing penalties or casting woes:

// BAD... calling Max(1, 2) implicitly boxes the parameters
// and the return value must be explicitly cast to int.
public static IComparable Max(IComparable a, IComparable b)
{
    return (a.CompareTo(b) > 0) ? a : b;
}

// Good! No boxing or casting. (Bonus use of the curiously recurring
// template pattern <http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern>.)
public static TComparable Max(TComparable a, TComparable b) where TComparable : IComparable<TComparable>
{
    return (a.CompareTo(b) > 0) ? a : b;
}

Despite all their benefits, parameters also have penalties and pitfalls. Consider this base class representing a worker thread with a typed state object:

public abstract class WorkerThread<TState>
{
    protected WorkerThread()
    {
    }

    public void Start(TState state)
    {
        /* ... */
    }

    public void Cancel()
    {
        /* ... */
    }

    protected abstract void Run(TState state);
}

Conveniently, an implementer of this class can directly and safely refer to the state object without needing to cast:

public class CompressionWorkerThread : WorkerThread<byte[]>
{
    /* ... */

    protected override void Run(byte[] state)
    {
        // . . . Compress the bytes here . . .
    }
}

public class ParserThread : WorkerThread<string>
{
    /* ... */

    protected override void Run(string state)
    {
        // . . . Parse the string here . . .
    }
}

But what is convenient for the implementer is a pain for someone who has to deal with many such objects. Because of the generic type parameter, we cannot do this:

private void CancelAll(IEnumerable<WorkerThread<???>> threads)
{
    foreach (WorkerThread<???> thread in threads)
    {
        thread.Cancel();
    }
}

There is no way to make this code compile since we must replace the ??? with a real type name. There is, however, a simple modification that will solve this problem. Let’s introduce a base interface which defines the Cancel method and have our WorkerThread<TState> derive from it:

public interface ICancellable
{
    void Cancel();
}

public abstract class WorkerThread<TState> : ICancellable
{
    /* ... */
}

Now we can implement CancelAll in terms of ICancellable instead:

private void CancelAll(IEnumerable<ICancellable> items)
{
    foreach (ICancellable item in items)
    {
        item.Cancel();
    }
}

This technique is sometimes referred to as type erasure. Check out Andrzej Krzemieński‘s posts on the topic for much more detail, albeit from a C++ perspective.

Embedded in this example is one additional parametric problem. Take a look at that Start method again:

public void Start(TState state)
{
    /* ... */
}

Yet again, if we needed to start a heterogeneous group of threads together, we would have difficulty due to the generically typed state parameter. But even if all threads were of the same type, we still wouldn’t be able to deal with them nicely. Here is an illustration of the problem:

private static void InitializeAndStart()
{
    List<CompressionWorkerThread> threads = new List<CompressionWorkerThread>()
    {
        new CompressionWorkerThread(),
        new CompressionWorkerThread(),
        /* ... */
    };

    List<byte[]> dataValues = new List<byte[]>()
    {
        new byte[] { /* ... */ },
        new byte[] { /* ... */ },
        /* ... */
    };

    StartCompressionThreads(threads, dataValues);
}

private static void StartCompressionThreads(IList<CompressionWorkerThread> threads, IList<byte[]> dataValues)
{
    for (int i = 0; i < threads.Count; ++i)
    {
        threads[i].Start(dataValues[i]);
    }
}

Yuck, parallel arrays! This is a breeding ground for bugs. We would have to be very careful that the lists are of the same length, that all data values are properly matched with a thread at the same index, and so on.

It is often of no use to allow this type of parameterization, especially when the expected use case calls for exactly one value to be passed. Instead, we should pass the values upon construction. We are already forced to be aware of the static type name here, so we don’t really lose anything by also directly associating the state object. The modified base class now looks like this:

public interface IWorker : ICancellable
{
    void Start();
}

public abstract class WorkerThread<TState> : IWorker
{
    protected WorkerThread(TState state)
    {
        /* ... */
    }

    public void Start()
    {
        /* ... */
    }

    public void Cancel()
    {
        /* ... */
    }

    protected abstract void Run(TState state);
}

The derived classes simply need to call the base constructor and pass along the state object:

public class CompressionWorkerThread : WorkerThread<byte[]>
{
    public CompressionWorkerThread(byte[] state)
        : base(state)
    {
    }

    /* ... */
}

The initialization code can now pass the state objects and return a collection of IWorker references:

private static IWorker[] CreateWorkers()
{
    return new IWorker[]
    {
        new CompressionWorkerThread(GetData1()),
        new CompressionWorkerThread(GetData2()),
        new ParserThread(GetString1()),
        // ...
    };
}

Finally, the manager is rewarded with a much simpler way to interact with workers:

private static void StartWorkers(IEnumerable<IWorker> workers)
{
    foreach (IWorker worker in workers)
    {
        worker.Start();
    }
}

Parameters have their place, but be cautious about the issues described above. Sometimes the best value is void!