Showing posts with label C#. Show all posts
Showing posts with label C#. Show all posts

Tuesday, December 18, 2007

Extension methods and primitive obsession

In another water-cooler argument today, a couple of coworkers didn't like my extension method example.  One main problem is that it violates instance semantics, where you expect that a method call off an instance won't work if the instance is null.  However, extension methods break that convention, leading the developer to question every method call and wonder if it's an extension method or not.  For example, you can run into these types of scenarios:

string nullString = null;

bool isNull = nullString.IsNullOrEmpty();

In normal circumstances, the call to IsNullOrEmpty would throw a NullReferenceException.  Since we're using an extension method, we leave it up to the developer of the extension method to determine what to do with null references.

Since there's no way to describe to the user of the API whether or not the extension method handles nulls, or how it handles null references, this can lead to quite a bit of confusion to clients of that API, or later, those maintaining code using extension methods.

In addition to problems with dealing with null references (which Elton pointed out, could be better handled with design-by-contract), some examples of extension methods online propose examples that show more than a whiff of the "Primitive Obsession" code smell:

Dealing with primitive obsession

In both of the examples above (Scott cites David's example), an extension method is used to determine if a string is an email:

string email = txtEmailAddress.Text;

if (! email.IsValidEmailAddress())
{
    // oh noes!
}

It's something I've done a hundred times, taking raw text from user input and performing some validation to make sure it's the "right" kind of string I want.  But where do you stop with validation?  Do you assume all throughout the application that this string is the correct kind of string, or do you duplicate the validation?

An alternative approach is accept that classes are your friend, and create a small class to represent your "special" primitive.  Convert back and forth at the boundaries between your system and customer-facing layers.  Here's the new Email class:

public class Email
{
    private readonly string _value;
    private static readonly Regex _regex = new Regex(@"^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$");

    public Email(string value)
    {
        if (!_regex.IsMatch(value))
            throw new ArgumentException("Invalid email format.", "value");

        _value = value;
    }

    public string Value
    {
        get { return _value; }
    }

    public static implicit operator string(Email email)
    {
        return email.Value;
    }

    public static explicit operator Email(string value)
    {
        return new Email(value);
    }

    public static Email Parse(string email)
    {
        if (email == null)
            throw new ArgumentNullException("email");

        Email result = null;

        if (!TryParse(email, out result))
            throw new FormatException("Invalid email format.");

        return result;
    }

    public static bool TryParse(string email, out Email result)
    {
        if (!_regex.IsMatch(email))
        {
            result = null;
            return false;
        }

        result = new Email(email);
        return true;
    }
}

I do a few things to make it easy on developers to use an email class that can play well with strings as well as other use cases:

  • Made Email immutable
  • Defined conversion operators to and from string
  • Added the Try-Parse pattern

The usage of the Email class closely resembles usage for other string-friendly types, such as DateTime:

string inputEmail = txtEmailAddress.Text;

Email email;

if (! Email.TryParse(inputEmail, out email))
{
    // oh noes!
}

txtEmailAddress.Text = email;

Now I can go back and forth from strings and my Email class, plus I provided a way to convert without throwing exceptions.  This looks very similar to code dealing with textual date representations.

Yes, but

The final Email class takes more code to write than the original extension method.  However, now that we have a single class that plays nice with primitives, additional Email behavior has a nice home.  With a class in place, I can now model more expressive emails, such as ones that include names like "Ricky Bobby <ricky.bobby@rb.com>".  Once the home is created, behavior can start moving in.  Otherwise, validation would be sprinkled throughout the system at each user boundary, such as importing data, GUIs, etc.

If you find yourself adding logic to primitives to the point of obsession, it's a strong indicator you're suffering from primitive obsession and a nice, small, specialized class can help eliminate a lot of the duplication primitive obsession tends to create.

Friday, December 14, 2007

Ruby-style Array methods in C# 3.0

A while back I played with Ruby-style loops in C# 3.0.  This sparked my jealousy of other fun Ruby constructs that I couldn't find in C#, and a couple of them are the "each" and "each_with_index" methods for arrays.  Here's an example, from thinkvitamin.com:

my_vitamins = ['b-12', 'c', 'riboflavin']

my_vitamins.each do |vitamin|
  puts "#{vitamin} is tasty!"
end
=> b-12 is tasty!
=> c is tasty!
=> riboflavin is tasty!

With both Arrays and List<T> in .NET, this is already possible: 

string[] myVitamins = {"b-12", "c", "riboflavin"};

Array.ForEach(myVitamins,
    (vitamin) =>
    {
        Console.WriteLine("{0} is tasty", vitamin);
    }
);

var myOtherVitamins = new List<string>() { "b-12", "c", "riboflavin" };

myOtherVitamins.ForEach(
    (vitamin) =>
    {
        Console.WriteLine("{0} is very tasty", vitamin);
    }
);

There are a few problems with these implementations, however:

  • Inconsistent between types
  • IEnumerable<T> left out
  • Array has a static method, whereas List<T> is instance
  • Index is unknown

Since T[] implicitly implements IEnumerable<T>, we can create a simple extension method to handle any case.

Without index

I still like the "Do" keyword in Ruby to signify the start of a block, and I'm not a fan of the readability (or "solubility", whatever) of the "ForEach" method.  Instead, I'll borrow from the loop-style syntax I created in the previous post that uses a "Do" method:

myVitamins.Each().Do(
    (vitamin) =>
    {
        Console.WriteLine("{0} is tasty", vitamin);
    }
);

To accomplish this, I'll need something to add the "Each" method, and something to provide the "Do" method.  Here's what I came up with:

public static class RubyArrayExtensions
{
    public class EachIterator<T>
    {
        private readonly IEnumerable<T> values;

        internal EachIterator(IEnumerable<T> values)
        {
            this.values = values;
        }

        public void Do(Action<T> action)
        {
            foreach (var item in values)
            {
                action(item);
            }
        }
    }

    public static EachIterator<T> Each<T>(this IEnumerable<T> values)
    {
        return new EachIterator<T>(values);
    }
}

The "Each" generic method is an extension method that extends anything that implements IEnumerable<T>, which includes arrays, List<T>, and many others.  IEnumerable<T> is ripe for extension, as .NET 3.5 introduced dozens of extension methods for it in the System.Linq.Enumerable class.  With these changes, I now have a consistent mechanism to perform an action against an array or list of items:

string[] myVitamins = { "b-12", "c", "riboflavin" };

myVitamins.Each().Do(
    (vitamin) =>
    {
        Console.WriteLine("{0} is tasty", vitamin);
    }
);

var myOtherVitamins = new List<string>() { "b-12", "c", "riboflavin" };

myOtherVitamins.Each().Do(
    (vitamin) =>
    {
        Console.WriteLine("{0} is very tasty", vitamin);
    }
);

With index

Ruby also has a "each_with_index" method for arrays, and in this case, there aren't any existing methods on System.Array or List<T> to accomplish this.  With extension methods, this is still trivial to accomplish.  I now just include the index whenever executing the callback to the Action<T, int> passed in.  Here's the extension method with the index:

public static class RubyArrayExtensions
{
    public class EachWithIndexIterator<T>
    {
        private readonly IEnumerable<T> values;

        internal EachWithIndexIterator(IEnumerable<T> values)
        {
            this.values = values;
        }

        public void Do(Action<T, int> action)
        {
            int i = 0;
            foreach (var item in values)
            {
                action(item, i++);
            }
        }
    }

    public static EachWithIndexIterator<T> EachWithIndex<T>(this IEnumerable<T> values)
    {
        return new EachWithIndexIterator<T>(values);
    }
}

The only difference here is I keep track of an index to send back to the delegate passed in from the client side, which now looks like this:

string[] myVitamins = { "b-12", "c", "riboflavin" };

myVitamins.EachWithIndex().Do(
    (vitamin, index) =>
    {
        Console.WriteLine("{0} cheers for {1}!", index, vitamin);
    }
);

var myOtherVitamins = new List<string>() { "b-12", "c", "riboflavin" };

myOtherVitamins.EachWithIndex().Do(
    (vitamin, index) =>
    {
        Console.WriteLine("{0} cheers for {1}!", index, vitamin);
    }
);

This now outputs:

0 cheers for b-12!
1 cheers for c!
2 cheers for riboflavin!
0 cheers for b-12!
1 cheers for c!
2 cheers for riboflavin!

Pointless but fun

I don't think I'd ever introduce these into production code, as it's never fun to drop new ways to loop on other's laps.  If anything, it shows how even parentheses can hinder readability, even if the method names themselves read better.

In any case, I now have a simple, unified mechanism to perform an action against any type that implements IEnumerable<T>, which includes arrays and List<T>.

Thursday, November 29, 2007

Hall of shame

We keep a "Hall of Shame" of WTF-level code snippets to remind us that however bad we might think things get, it could always be worse.  It also serves as a reminder to us that we can't be complacent with ignorance, and lack of strong technical leadership in critical applications can have far-reaching negative consequences.  Here are a few fun examples.

Runtime obsolescence

I've seen this when we have members that we don't want anyone to use anymore.  Unfortunately, it didn't quite come out right:

public static bool Authenticate(string username, string password)
{
    throw new Exception("I'm obsolete, don't call me, bro!");
}

Nobody knows not to call this method until their codebase actually executes this code at runtime, which could be at any phase in development.  If it's an obscure method, it might not get called at all until production deployment if test coverage is low.

We want to communicate to developers at compile-time that members are obsolete, and we can do this we the ObsoleteAttribute:

[Obsolete("Please use the SessionContext class instead.", true)]
public static bool Authenticate(string username, string password)
{
}

Both the IDE and compilers support this attribute to give feedback to developers when a member should no longer be used.

Exception paranoia

I've seen exception paranoia mostly when developers don't have a thorough understanding of the .NET Framework.  A try...catch block is placed around code that can absolutely never fail, but juuuuuust in case it does:

public static MyIdentity Ident
{
    get
    {
        if (((HttpContext.Current != null) && (HttpContext.Current.User != null)) && (HttpContext.Current.User.Identity != null))
        {
            try
            {
                return (HttpContext.Current.User.Identity as MyIdentity);
            }
            catch (Exception exception)
            {
                ExceptionManager.Publish(exception);
                return null;
            }
        }
        return null;
    }
}

This is also another case of TCPS.  The "if" block already takes care of any failures of the "try" block, and as we're using the "as" operator, the downcasting can never fail either.  The 13 lines of code above were reduced to only one:

public static MyIdentity Ident
{
    get
    {
        return HttpContext.Current.User.Identity as MyIdentity;
    }
}

If HttpContext.Current is null, I'd rather throw the exception, as my ASP.NET app probably won't work without ASP.NET up and running.

Clever debug switches

This is probably my favorite Hall of Shame example.  So much so that we named an award at my previous employer after the pattern.  Sometimes I like to insert a block of code for debugging purposes.  If I want my debug statements to live on in source control, I have a variety of means to do so, including compiler directives, the ConditionalAttribute, or even source control to look at history.  A not-so-clever approach we found was:

public void SubmitForm()
{
    if (false)
    {
        MessageBox.Show("Submit button was clicked.");
    }
}

To make the code run, you change the block to "if (true)".  This had us cracking up so much we named our Hall of Shame award after it, calling it the "If False Award".  Whenever we found especially heinous or corner-cutting code, they deserved the "If False Award".  We never awarded ignorance, only laziness.

Opportunities to improve

It's easy to let a "Hall of Shame" create negative and unconstructive views towards the codebase.  We always used it as a teaching tool to show that we would fix ignorance, but we would not tolerate laziness.

Every developer starts their career writing bad code.  Unfortunately, some never stop.

Intention-concealing interfaces: blob parameters

When someone is using your code, you want your code to be as explicit and easy to understand as possible.  This is achieved through Intention-Revealing Interfaces.  Evans describes the problems of opaque and misleading interfaces in Domain-Driven Design:

If a developer must consider the implementation of a component in order to use it, the value of encapsulation is lost.  If someone other than the original developer must infer the purpose of an object or operation based on its implementation, that new developer may infer a purpose that the operation or class fulfills only by chance.  If that was not the intent, the code may work for the moment, but the conceptual basis of the design will have been corrupted, and the two developers will be working at cross-purposes.

His recommendation is to create Intention-Revealing Interfaces:

Name classes and operations to describe their effect and purpose, without reference to the means by which they do what they promise.  This relieves the client developer of the need to understand the internals.  The name should conform to the Ubiquitous Language so that the team members can quickly infer their meaning.  Write a test for a behavior before creating it, to force your thinking into client developer mode.

Several times now I've run into blob parameters (I'm sure there are other names for them).  Blob parameters are amorphous, mystery parameters used in constructors and methods, usually of type ArrayList, HashTable, object[], or even params object[].  Here's an example:

public class OrderProcessor
{
    public void Process(object[] args)
    {
        Order order = (Order) args[0];
        Customer customer = (Customer) args[1];

        // etc.
    }
}

The intentions behind this code are good, but the results can lead to some frustrating results.  Furthermore, this pattern leads to "Intention-Concealing Interfaces", which is style completely opposite from the Intention-Revealing style Eric proposes.  Clients need to know the intimate details of the internal implementation of the component in order to use the blob parameters.

Wrong kind of extensibility

Blob parameters lead to the worst kind of coupling, beyond just "one concrete class using another concrete class".  When using blob parameters, the client gets coupled to the internal implementation of unwinding the blob.  Additionally, new client code must have intimate knowledge of the details, as the correct order of the parameters is not exposed by the blob method's interface.

When a developer needs to call the "Process" method above, they are forced to look at the internal implementation.  Hopefully they have access to the code, but otherwise they'll need to pop open Reflector to determine the correct parameters to pass in.  Instead of learning if the parameters are correct at compile-time, they have to wait until run-time.

Alternatives

Several alternatives exist before resorting to blob parameters:

  • Explicit parameters
  • Creation method
  • Factory patterns
  • IoC containers
  • Generics

Each of these alternatives can be used separately or combined together to achieve both Intention-Revealing Interfaces and simple code.

Explicit Parameters

Explicit parameters just means that members should ask explicitly what they need to operate through their signature.  The Process method would be changed to:

public class OrderProcessor
{
    public void Process(Order order, Customer customer)
    {
        // etc.
    }
}

The Process method takes exactly what components it needs to perform whatever operations it does, and clients of the OrderProcessor can deduce this simply through the method signature.

If this signature needs to change due to future requirements, you have a few choices to deal with this change:

  • Overload the method to preserve the existing signature
  • Just break the client code

People always assume it's a big deal to break client code, but in many cases, it's not.  Unless you're shipping public API libraries as part of your product, backwards compatibility is something that can be dealt with through Continuous Integration.

Creation Method/Factory Patterns/IoC Containers

When blob parameters start to invade constructors, it's a smell that you need some encapsulation around object creation.  Instead of dealing with changing requirements through blob parameters, encapsulate object creation:

public class OrderProcessor
{
    private readonly ISessionContext _context;

    public OrderProcessor() : this(ObjectFactory.GetInstance<ISessionContext>())
    {
    }

    public OrderProcessor(ISessionContext context)
    {
        _context = context;
    }
}

Originally, the ISessionContext was a hodgepodge of different object that I needed to pass in to the OrderProcessor class.  Since these dependencies became more complex, I encapsulated them into a parameter object, and introduced a factory method (the "ObjectFactory" class) to encapsulate creation.  Client code no longer needs to pass in a group of complex objects to create the OrderProcessor.

Generics

Sometimes blob parameters surface because a family of objects needs to be created or used, but all have different needs.  For example, the Command pattern that requires data to operate might look like this before generics:

public interface ICommand
{
    void Execute(object[] args);
}

public class TransferCommand : ICommand
{
    public void Execute(object[] args)
    {
        Account source = (Account) args[0];
        Account dest = (Account) args[1];
        int amount = (int) args[2];

        source.Balance -= amount;
        dest.Balance += amount;
    }
}

The ICommand interface needs to be flexible in the arguments it can pass to specific implementations.  We can use generics instead of blob parameters to accomplish the same effect:

public interface ICommand<T>
{
    void Execute(T args);
}

public struct Transaction
{
    public Account Source;
    public Account Destination;
    public int Amount;

    public Transaction(Account source, Account destination, int amount)
    {
        Source = source;
        Destination = destination;
        Amount = amount;
    }
}

public class TransferCommand : ICommand<Transaction>
{
    public void Execute(Transaction args)
    {
        Account source = args.Source;
        Account dest = args.Destination;
        int amount = args.Amount;

        source.Balance -= amount;
        dest.Balance += amount;
    }
}

Each ICommand implementation can describe its needs through the generic interface, negating the need for blob parameters.

No blobs

I've seen a lot of misleading, opaque code, but blob parameters take the prize for "Intention-Concealing Interfaces".  Instead of components being extensible, they're inflexible, brittle, and incomprehensible.  Before going down the path of creating brittle interfaces, exhaust all alternatives before doing so.  Every blob parameter I've created I rolled back later as it quickly becomes difficult to deal with.

Monday, November 26, 2007

String extension methods

I got really tired of the IsNullOrEmpty static method, so I converted it into an extension method:

public static class StringExtensions
{
    public static bool IsNullOrEmpty(this string value)
    {
        return string.IsNullOrEmpty(value);
    }
}

Since that method was already static, I just put a simple wrapper around it.  All scenarios pass:

[TestMethod]
public void Matches_existing_behavior()
{
    Assert.IsFalse("blarg".IsNullOrEmpty());
    Assert.IsTrue(((string)null).IsNullOrEmpty());
    Assert.IsTrue("".IsNullOrEmpty());

    string value = null;

    Assert.IsTrue(value.IsNullOrEmpty());

    value = string.Empty;

    Assert.IsTrue(value.IsNullOrEmpty());

    value = "blarg";

    Assert.IsFalse(value.IsNullOrEmpty());
}

The only rather strange thing about extension methods is that they can be called by null references, as shown in the test above.  As long as everyone understands that extension methods are syntactic sugar and not actually extending the underlying types, I think it's clear enough.

The extension method is more readable and lets me work with strings instead of having to remember that it's a static method.  Static methods that reveal information about instances of the same type seem to be prime candidates for extension methods.

Wednesday, October 24, 2007

Entity validation with visitors and extension methods

On the Yahoo ALT.NET group, an interesting conversation sprung up around the topic of validation.  Entity validation can be a tricky beast, as validation rules typically depend on the context of the operation (persistence, business rules, etc.).

In complex scenarios, validation usually winds up using the Visitor pattern, but that pattern can be slightly convoluted to use from client code.  With extension methods in C# 3.0, the Visitor pattern can be made a little easier.

Some simple validation

In our fictional e-commerce application, we have a simple Order object.  Right now, all it contains are an identifier and the customer's name that placed the order:

public class Order
{
    public int Id { get; set; }
    public string Customer { get; set; }
}

Nothing too fancy, but now the business owner comes along and requests some validation rules.  Orders need to have an ID and a customer to be valid for persistence.  That's not too hard, I can just add a couple of methods to the Order class to accomplish this.

The other requirement is to have a list of broken rules in case the object isn't valid, so the end user can fix any issues.  Here's what we came up with:

public class Order
{
    public int Id { get; set; }
    public string Customer { get; set; }

    public bool IsValid()
    {
        return BrokenRules().Count() > 0;
    }

    public IEnumerable<string> BrokenRules()
    {
        if (Id < 0)
            yield return "Id cannot be less than 0.";

        if (string.IsNullOrEmpty(Customer))
            yield return "Must include a customer.";

        yield break;
    }
}

Still fairly simple, though I'm starting to bleed other concerns into my entity class, such as persistence validation.  I'd rather not have persistence concerns mixed in with my domain model, it should be another concern altogether.

Using validators

Right now I have one context for validation, but what happens when the business owner requests display validation?  In addition to that, my business owner now has a black list of customers she won't sell to, so now I need to have a black list validation, but that's really separate from display or persistence validation.  I don't want to keep adding these different validation rules to Order, as some rules are only valid in certain contexts.

One common solution is to use a validation class together with the Visitor pattern to validate arbitrary business/infrastructure rules.  First, I'll need to define a generic validation interface, as I have lots of entity classes that need validation (Order, Quote, Cart, etc.):

public interface IValidator<T>
{
    bool IsValid(T entity);
    IEnumerable<string> BrokenRules(T entity);
}

Some example validators might be "OrderPersistenceValidator : IValidator<Order>", or "CustomerBlacklistValidator : IValidator<Customer>", etc.  With this interface in place, I modify the Order class to use the Visitor pattern.  The Visitor will be the Validator, and the Visitee will be the entity class:

public interface IValidatable<T>
{
    bool Validate(IValidator<T> validator, out IEnumerable<string> brokenRules);
}

public class Order : IValidatable<Order>
{
    public int Id { get; set; }
    public string Customer { get; set; }

    public bool Validate(IValidator<Order> validator, out IEnumerable<string> brokenRules)
    {
        brokenRules = validator.BrokenRules(this);
        return validator.IsValid(this);
    }
}

I also created the "IValidatable" interface so I can keep track of what can be validated and what can't.  The original validation logic that was in Order is now pulled out to a separate class:

public class OrderPersistenceValidator : IValidator<Order>
{
    public bool IsValid(Order entity)
    {
        return BrokenRules(entity).Count() > 0;
    }

    public IEnumerable<string> BrokenRules(Order entity)
    {
        if (entity.Id < 0)
            yield return "Id cannot be less than 0.";

        if (string.IsNullOrEmpty(entity.Customer))
            yield return "Must include a customer.";

        yield break;
    }
}

This class can now be in a completely different namespace or assembly, and now my validation logic is completely separate from my entities.

Extension method mixins

Client code is a little ugly with the Visitor pattern:

Order order = new Order();
OrderPersistenceValidator validator = new OrderPersistenceValidator();

IEnumerable<string> brokenRules;
bool isValid = order.Validate(validator, out brokenRules);

It still seems a little strange to have to know about the correct validator to use.  Elton wrote about a nice trick with Visitor and extension methods that I could use here.  I can use an extension method for the Order type to wrap the creation of the validator class:

public static bool ValidatePersistence(this Order entity, out IEnumerable<string> brokenRules)
{
    IValidator<Order> validator = new OrderPersistenceValidator();

    return entity.Validate(validator, brokenRules);
}

Now my client code is a little more bearable:

Order order = new Order();

IEnumerable<string> brokenRules;
bool isValid = order.ValidatePersistence(out brokenRules);

My Order class doesn't have any persistence validation logic, but with extension methods, I can make the client code unaware of which specific Validation class it needs.

A generic solution

Taking this one step further, I can use a Registry to register validators based on types, and create a more generic extension method that relies on constraints:

public class Validator
{
    private static Dictionary<Type, object> _validators = new Dictionary<Type, object>();

    public static void RegisterValidatorFor<T>(T entity, IValidator<T> validator)
        where T : IValidatable<T>
    {
        _validators.Add(entity.GetType(), validator);
    }

    public static IValidator<T> GetValidatorFor<T>(T entity)
        where T : IValidatable<T>
    {
        return _validators[entity.GetType()] as IValidator<T>;
    }

    public static bool Validate<T>(this T entity, out IEnumerable<string> brokenRules)
        where T : IValidatable<T>
    {
        IValidator<T> validator = Validator.GetValidatorFor(entity);

        return entity.Validate(validator, out brokenRules);
    }
}

Now I can use the extension method on any type that implements IValidatable<T>, including my Order, Customer, and Quote classes.  In my app startup code, I'll register all the appropriate validators needed.  If types use more than one validator, I can modify my registry to include some extra location information.  Typically, I'll keep all of this information in my IoC container so it can all get wired up automatically.

Visitor patterns are useful when they're really needed, as in the case of entity validation, but can be overkill sometimes.  With extension methods in C# 3.0, I can remove some of the difficulties that Visitor pattern introduces.

Monday, October 15, 2007

Ruby-style loops in C# 3.0

Ruby has a pretty interesting (and succinct) way of looping through a set of numbers:

5.times do |i|
  print i, " "
end

The results of executing this Ruby block is:

0 1 2 3 4

I really love the readability and conciseness of this syntax, just enough to see what's going on, but not a lot of extra stuff to get in the way.  In addition to the "times" method, there's also "upto" and "downto" methods for other looping scenarios.

Some slight of hand

With extension methods and lambda expressions and C# 3.0, this form of loop syntax is pretty easy to do.  Not really a great idea, but at least an example of what these new constructs in C# 3.0 can do.

So how can we create this syntax in C#?  The "times" method is straightforward, that can just be an extension method for ints:

public static void Times(this int count)

Now this method shows up in IntelliSense (when I add the appropriate "using" directive):

Now that the Times method shows up for ints, we can focus on the Do block.

Adding the loop behavior

Although I can't declare blocks in C# 3.0, lambda expressions are roughly equivalent.  To take advantage of lambda expressions, I want to give the behavior to the Times method in the form of a delegate.  By declaring a delegate parameter type on a method, I'm able to use lambda expressions when calling that method.

I didn't like passing the lambda directly to the "Times" method, so I created an interface to encapsulate a loop iteration, faking the Ruby "do" block with methods:

public interface ILoopIterator
{
    void Do(Action action);
    void Do(Action<int> action);
}

Now I can return an ILoopIterator from the Times method instead of just "void".  Now the final part is to create an "ILoopIterator" implementation that will do the actual looping:

private class LoopIterator : ILoopIterator
{
    private readonly int _start, _end;

    public LoopIterator(int count)
    {
        _start = 0;
        _end = count - 1;
    }

    public LoopIterator(int start, int end)
    {
        _start = start;
        _end = end;
    }  

    public void Do(Action action)
    {
        for (int i = _start; i <= _end; i++)
        {
            action();
        }
    }

    public void Do(Action<int> action)
    {
        for (int i = _start; i <= _end; i++)
        {
            action(i);
        }
    }
}

public static ILoopIterator Times(this int count)
{
    return new LoopIterator(count);
}

I let the "LoopIterator" class encapsulate the behavior of performing the underlying "for" loop and calling back to the Action passed in as a lambda.  It makes more sense when you see some client code calling the Times method:

int sum = 0;
5.Times().Do( i => 
    sum += i
);
Assert.AreEqual(10, sum);

That looks pretty similar to the Ruby version (but not quite as nice), but it works.  Compare this to a normal loop in C#:

int sum = 0;
for (int i = 0; i < 5; i++)
{
    sum += i;
}
Assert.AreEqual(10, sum);

Although the "for" syntax is functional and about the same number of lines of code, the Ruby version is definitely more readable.  Adding additional UpTo and DownTo methods would be straightforward with additional ILoopIterator implementations.

Feature abuse

Yeah, I know this is more than a mild case of feature abuse, but it was interesting to see the differences between similar operations in C# 3.0 and Ruby.  Although it's possible to do these similar operations, with similar names, this example highlights how much the syntax elements of the static CLR languages can get in the way of a readable API, and how much Ruby stays out of the way.

Friday, October 12, 2007

Double-edged sword of InternalsVisibleTo

I've had some conversations with both Joe and Elton lately about the InternalsVisibleTo attribute.  From the documentation, the assembly-level InternalsVisibleTo attribute:

Specifies that all nonpublic types in an assembly are visible to another assembly.

This attribute was introduced in C# 2.0, and allows you to specify other assemblies that can see all types and members marked "internal".  In practice, all assemblies are signed with the same public key, and you'd specify that the unit test assembly can see that assembly.

  • MyProject.Core -> has the "InternalsVisibleTo" attribute defined in the AssemblyInfo.cs file, pointing at the below assembly
  • MyProject.Core.Tests -> is signed with the same public key as the Core assembly

Notice that the "Core" project knows about the "Tests" project, but the actual project dependency is the other way around.  It's definitely better than using reflection to access private members for testing, but there are some definite pros and cons with this approach.

Pros

  • Allows your test libraries to access internal classes and methods for additional testing and coverage
  • Keeps your public API limited to what you want to publish
  • Provides greater flexibility for internal refactoring and backwards compatibility
  • Reduces the surface area of your public API

Cons

  • Easily abused, so things usually marked "private" are now marked "internal"
  • Potential loss of encapsulation
  • Decision about what should be public could be wrong
  • Essentially two levels of "public" visibility that have to be managed
  • Enforces bi-directional dependencies between assemblies

Personally, I always felt like marking something "internal" was cheating just a little bit, and I have trouble deciding when to make something internal or not.  But unless you're delivering a public, published and documented API as part of your product, using the "InternalsVisibleTo" attribute would probably be overkill.

However, if you are delivering an API, you should consider using this attribute to keep a high level of coverage and reduce the surface area the API for your customers.  You could try starting by making everything "internal", then shape the public API based on specific use cases.

Friday, August 31, 2007

Legacy code testing techniques: subclass and override non-virtual members

One of the core techniques in Michael Feathers' Working Effectively With Legacy Code is the "Subclass and Override Method" technique.  Basically, in the context of a test, we can subclass a dependency and override behavior on a method or property to nullify or alter its behavior as needed.  This is especially useful if we can't extract a dependency outside of a class under test.  Mocking frameworks such as Rhino.Mocks allow me to verify interactions, or simply put in some canned responses for dependencies.

Since we're dealing with legacy code, paying down the entire technical debt at once isn't an option.  We have to make micro-payments by introducing seams into our codebase.  It might make the code slightly uglier at first, but as Feathers notes in his book, sometimes surgery leaves scars.

Some problem code

I need to override a method to provide an alternate, canned value.  However, in C#, members are not virtual by default.  Members have to be explicitly marked "virtual", otherwise subclassed members can only shadow base members.  For instance, suppose we have class Foo and Bar:

public class Foo : Bar
{
    public decimal CalculateDiscount()
    {
        return CalculateTotal() * 0.1M;
    }
}

public class Bar
{
    public decimal CalculateTotal()
    {
        // Hit database, webservices, etc.
        return PricingDatabase.GetPrice(itemId);
    }
}

I'm trying to test some class that uses Foo.CalculateDiscount method to perform some figure out what shipping methods should be available.  It uses the Foo discount to figure this out.  However, CalculateDiscount calls a base class method (CalculateTotal), which then goes and hits the database!  Not much of a unit test when it hits the database (or web service, or HttpContext, etc.).  Worse, I don't have access to the Bar class, it's in another library that we don't own.  Here's the test I'm trying to write:

[Test]
public void Should_add_shipping_option_when_discount_is_lower_than_50()
{
    MockRepository mocks = new MockRepository();

    Foo foo = mocks.CreateMock<Foo>();

    using (mocks.Record())
    {
        Expect.Call(foo.CalculateTotal()).Return(100.0M);
    }

    using (mocks.Playback())
    {
        string[] shippingMethods = ShippingService.FindShippingMethods(foo);

        Assert.AreEqual(shippingMethods[0], "OneDay");
    }
}

I could try to remove the dependency on the Foo class somehow.  But there's a lot of information that my shipping method needs, so I can't just pass in individual parameters for all of the Foo data.  The Foo class is actually quite large, so trying to extract an interface wouldn't really help either.  In any case, I just want to override the behavior of the "CalculateTotal" call, and since it's not marked "virtual", I can't override the behavior directly.  For example, this won't work in my test:

public class FooStub : Foo
{
    public new decimal CalculateTotal()
    {
        return 100.0m;
    }
}

The shipping service knows about "Foo", not "FooStub".  Since the FooStub.CalculateTotal is shadowing the Bar method, only when I have a variable of type FooStub will its CalculateTotal get called.  Since the shipping service knows Foo, it will use the Bar CalculateTotal method, even if I pass in an instance of a FooStub.  Shadowing can cause weird things like this to happen, to I try to avoid it as much as possible.

Subclass and override

So subclassing and overriding, one of the main functions of mocking frameworks, won't work at the test level.  I need to get the override on or between the Foo and Bar classes.  Why don't we create a new BarSeam class between the Foo and Bar classes?

public class Foo : BarSeam
{
    public decimal CalculateDiscount()
    {
        return CalculateTotal() * 0.1M;
    }
}

public class BarSeam : Bar
{
    public virtual new decimal CalculateTotal()
    {
        return base.CalculateTotal();
    }
}

public class Bar
{
    public decimal CalculateTotal()
    {
        // Hit database, webservices, etc.
        return PricingDatabase.GetPrice(itemId);
    }
}

The BarSeam class now sits between Foo and Bar in the inheritance hierarchy.  BarSeam shadows the Bar.CalculateTotal, with one key addition, the "virtual" keyword.  By making BarSeam.CalculateTotal virtual, I can now subclass Foo and override CalculateTotal, put in a canned response, and not worry about hitting the database.

Yes, BarSeam is ugly, but I want to bring ugly to the front of the class to make sure it doesn't get swept under the rug.  Since all client code either references Foo or Bar, no client code will be affected as BarSeam's default implementation merely calls the base method.  Think of it as Adapter, but instead of incompatible interfaces, I'm dealing with a non-extensible interface.

Conclusion

When working with legacy code, it's not feasible (or even wise) to try and rewrite the application.  By making these micro-refactorings and introducing seams, we're able to identify places in need of larger refactorings, as well as meeting our original goal of getting our legacy code under test.

Wednesday, August 29, 2007

Template Delegate Pattern

I've had to use this pattern a few times, most recently in Behave#.  It's similar to the Template Method pattern, but doesn't resort to using subclassing for using a template method.  Instead, a delegate is passed to the Template Method to substitute different logic for portions of the algorithm.

The pattern

Two methods in unrelated classes perform similar general algorithms, yet some parts of the algorithm are different.

Generalize the algorithm by extracting their steps into a new class, then extract methods for the specialized parts into delegates to be passed in to the new class.

Motivation

Helper methods tend to clutter a class with responsibilities orthogonal to its true purpose.  Additionally, several methods in one class might need to perform the same algorithm in slightly different ways.  While Template Method is concerned with removing behavioral duplication, algorithmic duplication can be just as rampant. 

Removing algorithmic duplication through Template Method can make the code more obtuse, as often the abstraction of the template class doesn't add any meaning to the system.  Additionally, sometimes duplicate algorithms are in completely separate classes, and it would be impossible or unwise to try to extract a common subclass from the two classes.  We can remove algorithmic duplication by consolidating the algorithm into its own method or class, and pass in parts of the algorithm that might vary.

Mechanics

  • Use Extract Method to separate the purely algorithmic section of each method from any surrounding behavioral code.
  • Compile and test.
  • Decompose the methods in each algorithm so that all of the steps in the algorithm are identical or completely different.
  • For each step in the algorithm that is completely different, use Extract Method to pull out the varying logic.  Name these extracted methods the same.
  • Compile and test.
  • Optionally, use Introduce Parameter Object for each extracted algorithm method that does not match the same number of parameters for the other different algorithm steps.  Compile and test.
  • If there is not an existing delegate type that matches the extracted variant methods of the algorithm, create a delegate type that matches both extracted methods.
  • Use Add Parameter and add a new parameter of the delegate type created earlier, and modify the algorithm to use the delegate method to execute the varying logic.  Repeat for both algorithm methods.  Each algorithm method should look exactly the same at this point, except for parameter and return types.
  • Compile and test.
  • Use Extract Class and Move Method to move the two algorithm methods to a new, generalized (and optionally generic) class.  Modify the calling class to use the new class and methods.
  • Compile and test.

Example

Suppose I find the two methods in different classes in a large codebase that do recursive searches.  One finds a Control based on an ID, and the other searches for XmlNodes based on an attribute value:

private Control FindControl(ControlCollection controls)
{
    foreach (Control control in controls)
    {
        if (control.ID == txtControlID.Text)
            return control;

        return FindControl(control.Controls);
    }
}

private XmlNode FindElement(XmlNodeList nodes)
{
    foreach (XmlNode node in nodes)
    {
        if (node.Attributes["ID"].Value == "4564")
            return node;

        return FindElement(node.ChildNodes);
    }
}

Both of these methods perform the exact same logic, a recursive search, but the details are slightly different.

In this example, the first step is already complete and each method contains only the algorithm I'm interested in.  From looking at these methods, it looks like there are 3 distinct parts: the loop, the comparison, and the actual matching logic.  The two differing parts I see in the algorithm are:

  • Match
  • Get the children based on the current item in the loop

I'll apply Extract Method to pull out the varying logic and name these methods the same.  Here are the extracted methods:

private bool IsMatch(Control control)
{
    return control.ID == txtControlID.Text;
}

private bool IsMatch(XmlNode node)
{
    return node.Attributes["ID"].Value == "4564";
}

private ControlCollection GetChildren(Control control)
{
    return control.Controls;
}

private XmlNodeList GetChildren(XmlNode node)
{
    return node.ChildNodes;
}

Note that the names of each method is the same, as well as the number of parameters.  Now the original algorithm methods call these extracted varying methods:

private Control FindControl(ControlCollection controls)
{
    foreach (Control control in controls)
    {
        if (IsMatch(control))
            return control;

        return FindControl(GetChildren(control));
    }
}

private XmlNode FindElement(XmlNodeList nodes)
{
    foreach (XmlNode node in nodes)
    {
        if (IsMatch(node))
            return node;

        return FindElement(GetChildren(node));
    }
}

These algorithm methods are starting to look very similar.  Next, I need to find a delegate type to represent the varying methods, namely "IsMatch" and "GetChildren".  Since I'm working with Visual Studio 2008, some good candidates already exist with the Func delegate types.  I like these delegate types as they are generic and may lend to some better algorithm definitions in the future, so I'll stick with Func.  Here's the FindControl method after I use Add Parameter to pass in the varying algorithm logic:

private Control FindControl(IEnumerable<Control> controls, 
    Func<Control, bool> predicate,
    Func<Control, IEnumerable<Control>> childrenSelector)
{
    foreach (Control control in controls)
    {
        if (predicate(control))
            return control;

        return FindControl(childrenSelector(control), predicate, childrenSelector);
    }
}

I changed the type of the "controls" parameter to IEnumerable<Control> from ControlCollection, to reduce the number of types seen in the algorithm method.  I change the client code of this algorithm to pass in the new parameters, compile and test:

private void SetLabelText()
{
    string text = txtLabelText.Text;
    string controlID = txtControlID.Text;

    if (string.IsNullOrEmpty(text) || string.IsNullOrEmpty(controlID))
        return;

    Control control = FindControl(page.Controls, IsMatch, GetChildren);
}

Note that the "IsMatch" and "GetChildren" are method names in this class, so I'm passing the matching and children algorithms to the FindControl method for execution when needed.  Finally, I use Extract Class and Move Method to move the two virtually identical algorithm methods to a single method on a new class.  Here's the final result, with some minor changes to use extension methods:

public static T RecursiveSearch<T>(this IEnumerable<T> items,
    Func<T, bool> predicate,
    Func<T, IEnumerable<T>> childrenSelector)
{
    foreach (T item in items)
    {
        if (predicate(item))
            return item;

        return RecursiveSearch(childrenSelector(item), predicate, childrenSelector);
    }

    return default(T);
}
 

I've made the method generic, as the only final variant between the extracted algorithm methods was the type of the item I was finding.  By making the method generic, the return type is strongly typed to the item I'm searching for.  The final client code doesn't look too much different from earlier:

private void SetLabelText()
{
    string text = txtLabelText.Text;
    string controlID = txtControlID.Text;

    if (string.IsNullOrEmpty(text) || string.IsNullOrEmpty(controlID))
        return;

    Control control = page.Controls.Cast<Control>().RecursiveSearch(IsMatch, GetChildren);
}

The client code for the "FindElement" looks exactly the same, and now there is no duplication of the recursive search logic.  I've extracted the varying logic into methods which can be passed in as delegates to the new, generic extracted algorithm.  Since I'm using C# 3.0, I can go as far as using lambda expressions instead of the extracted "IsMatch" and "GetChildren" methods.

Conclusion

The Template Delegate pattern is used quite extensively with the new Enumerable extension methods (such as the Where method).  With delegate creation in C# 3.0 becoming much simpler with lambda expressions, it's becoming easier to compose generalized algorithms where varying portions are passed in as delegate parameters.  By using the Template Delegate pattern, I can reduce the number of junk algorithmic methods into a single generic class that serves the needs of current and future client code.

Friday, July 20, 2007

Constrained generic extension methods

When I first saw extension methods, a new feature in C# 3.0, I was a bit skeptical.  It seemed like yet another language feature shoehorned in to support LINQ.  After going a few rounds with the technology, it actually looks quite promising.  For a full explanation of extension methods and what scenarios they can enable, check out Scott Guthrie's post on the subject.

Making the extension method generic

First, let's examine the original example from Scott's post, which adds an "In" method to test whether an item is in a collection:

public static bool In(this object o, IEnumerable items)
{
    foreach (object item in items)
    {
        if (item.Equals(o))
            return true;
    }

    return false;
}

The "In" method will be added to all objects, including primitive value types such as "int" and "double".  Here's a sample snippet using "int":

int[] values = {5, 6, 10};

bool isInArray = 7.In(values);

The value "isInArray" evaluates to "false", as the number 7 isn't in the array.  So what's wrong with this code?  The problem is that the "In" method uses the type "object" to extend types, but that will cause boxing when the number 7 is boxed to an object for the extension method call.  Generics can prevent boxing from occurring, so let's change the extension method to be generic:

public static bool In<T>(this T o, IEnumerable<T> items)
{
    foreach (T item in items)
    {
        if (item.Equals(o))
            return true;
    }

    return false;
}

Now when I call "In", the method will use "int" instead of "object", and no boxing will occur.  Additionally, since I used the generic IEnumerable<T> type, I will get some additional compile-time validation, so I won't be able to do things like this:

string[] values = {"5", "6", "10"};

bool isInArray = 7.In(values); // Compile time error!

I can now lean on the compiler to do some type checking for me.  What about some more interesting scenarios, where I want to extend some complex types?

Adding constraints to a generic extension method

I'd like to add some complex comparison operators to certain types, say something like "IsLessThanOrEqualTo".  I'd like to extend types that implement both IComparable<T> and IEquatable<T>.  That is, the extension method should only show up for types that implement both interfaces.  I can't declare both types with the "this" modifier parameter.  How can I accomplish this?  The trick is to make the method generic, and add constraints:

public static bool IsLessThanOrEqualTo<T>(this T lValue, T value)
    where T : IComparable<T>, IEquatable<T>
{
    return lValue.CompareTo(value) < 0 || lValue.Equals(value);
}

This extension method lets me write code such as:

int x = 5;
int y = 10;

bool isLte = x.IsLessThanOrEqualTo(y);
Assert.IsTrue(isLte);

This gives me a few advantages:

  • The available constraints are the same as any other generic type or method (struct, class, new(), <base class>, <interface>, and naked type constraints)
  • Using multiple constraints lets me constrain the method to types that fulfill all the constraints, such as the two interfaces in the above example
  • The signature of type T inside the generic method combines the members of all of the constraints.
    • All of the methods of IComparable<T> and IEquatable<T> are available to me in the above example.
  • The IDE filters the constraints in IntelliSense, and I won't even see extension methods whose constraints my variable doesn't fulfill

What this allows me to do in real world situations is to target specific scenarios by filtering through constraints.  Instead of using only 1 type in the "this" parameter, I can target objects that derive from a certain class, implement several interfaces, etc.  By making the class generic, interactions with the method will be strongly typed and will avoid boxing conversions and unnecessary casting.

To see the constraints fail, what happens when we try to call the IsLessThanOrEqualTo with a class that only partially fulfills the constraints?  Here's the bare-bones class:

public class Account : IComparable<Account>
{
    public int CompareTo(Account other)
    {
        return 0;
    }
}

I try to compile the following code:

Account account = new Account();
Account other = new Account();

account.IsLessThanOrEqualTo(other); // Compile time error!

It won't compile, since Account only implements IComparable<T>, and not IEquatable<T>.  This wouldn't happen while authoring the code, as IntelliSense doesn't even show the IsLessThanOrEqualTo method.

I should note that if I don't constrain the generic extension method, any instance of any type will have the method available for use, which may or may not be desirable.

Conclusion

Generic types and methods can be very helpful in eliminating boxing and unboxing as well as casting that clutters up the code.  Extension methods allow me to add functionality to types that I may not have access to changing.  By combining generic, constrained methods and extension methods, I get the power of extension methods with the flexibility of generic methods.

Thursday, July 19, 2007

LINQ to Reflection

One of the great things about LINQ is that it allows me to query over any object that implements IEnumerable<T>.  This includes arrays, List<T>, Collection<T>, and many others.  Since many operations involving reflection return arrays, this means I'm open to using LINQ over those operations.  Let's look at a few examples.

Loading up assemblies for searching

First, I'd like to load some assemblies for searching.  The easiest way I found to do this was to use System.Reflection.Assembly.LoadWithPartialName.  This method has been deprecated as of .NET 2.0, but it will suffice for simple operations such as searching.  I wouldn't use that method for production code, it's too unreliable.

I first came up with a list of assembly names I wanted to search and dumped them in an array:

string[] assemblyNames = { "System", 
                 "mscorlib", 
                 "System.AddIn",
                 "System.Configuration", 
                 "System.Core", 
                 "System.Data",
                 "System.Data.Entity",
                 "System.Data.Entity.Design",
                 "System.Data.Linq",
                 "System.Deployment",
                 "System.Design",
                 "System.DirectoryServices",
                 "System.DirectoryServices.Protocols",
                 "System.Drawing",
                 "System.Drawing.Design",
                 "System.EnterpriseServices",
                 "System.IdentityModel",
                 "System.IdentityModel.Selectors",
                 "System.Management",
                 "System.Management.Instrumentation",
                 "System.Messaging",
                 "System.Printing",
                 "System.Runtime.Remoting",
                 "System.Runtime.Serialization",
                 "System.Security",
                 "System.ServiceModel",
                 "System.ServiceProcess",
                 "System.Transactions",
                 "System.Web", 
                 "System.Web.Services", 
                 "System.Windows.Forms", 
                 "System.Workflow.Activities", 
                 "System.Workflow.ComponentModel", 
                 "System.Workflow.Runtime", 
                 "System.WorkflowServices", 
                 "System.Xml", 
                 "System.Xml.Linq"
             };

It's pretty much all of the .NET Framework 3.5 assemblies.  Next, I want to create a LINQ query to load up all of the assemblies, so I can perform searches over those assemblies.  Just using the extension methods, it would look like this:

var assemblies = assemblyNames.Select(name => Assembly.LoadWithPartialName(name));

With LINQ magic, here's what the same query would look like:

var assemblies = from name in assemblyNames
            select Assembly.LoadWithPartialName(name);

With the "assemblies" variable (which is actually IEnumerable<Assembly>), I can start performing queries over the loaded assemblies.

Finding generic delegates

I was experimenting with an API, and I wanted to know if a generic delegate I created already existed somewhere in .NET.  I can use some handy LINQ expressions over the loaded assemblies to do just that.  But first, I need to know how to find what I'm looking for.  I can just load up a generic delegate I'm already aware of into a Type object:

Type actionDelegate = typeof(Action<>);

When I load up a generic delegate into an instance of a Type object, I notice some key properties, such as "IsGenericType" and "IsPublic".  Both of these are true for my generic delegate type.  Unfortunately, there is no "IsDelegate" property, but it turns out that IsSubclassOf(typeof(Delegate)) will return "true".  Combining these three conditions, I have a predicate to use in my search across the assemblies.

Here's the final LINQ query:

var types = from name in assemblyNames
            select Assembly.LoadWithPartialName(name) into a
            from c in a.GetTypes()
            where c.IsGenericType && c.IsPublic && c.IsSubclassOf(typeof(Delegate))
            select c;

foreach (Type t in types)
{
    Debug.WriteLine(t);
}

This is actually two LINQ queries joined together with a continuation (the "into a" part).  The first query enumerates over the assembly string names to load the assemblies.  The second query calls "GetTypes" on each assembly to load the types, and uses the "where" clause to only pull out the generic delegates using a predicate I found earlier.  The results of this block of code shows me the generic delegates:

System.EventHandler`1[TEventArgs]
System.Action`1[T]
System.Comparison`1[T]
System.Converter`2[TInput,TOutput]
System.Predicate`1[T]
System.Linq.Func`1[TResult]
System.Linq.Func`2[TArg0,TResult]
System.Linq.Func`3[TArg0,TArg1,TResult]
System.Linq.Func`4[TArg0,TArg1,TArg2,TResult]
System.Linq.Func`5[TArg0,TArg1,TArg2,TArg3,TResult]

Not a whole lot, but it did tell me what was already there.  Specifically, I had been looking for alternate overloads to Action<T>, to see if there were multiple delegate declarations like there are for Func<TResult>.  There aren't, but it turns out there are planned overloads for Action to match Func.

Context and IDisposable

I had a discussion with a team member that centered around types with names postfixed with "Context".  I was arguing that types named "Context" implied that the type implemented IDisposable, as it was intended to create a scope.  Instead of arguing in conjectures, I sought to get some actual data, and find how many types in the .NET Framework named "Context" also implemented IDisposable.  Here's the LINQ query I came up with:

var types = from name in assemblyNames
            select Assembly.LoadWithPartialName(name) into a
            from c in a.GetTypes()
            where (c.IsClass || c.IsInterface) && c.FullName.EndsWith("Context")
            group c.FullName by c.GetInterfaces().Contains(typeof(IDisposable)) into g
            select new { IsIDisposable = g.Key, Types = g };

In this query, I want to select all types that the name ends with "Context", and group these into different bins based on whether they implement IDisposable or not.  To do this, I take advantage of both grouping in LINQ, and anonymous types to hold the data.  I output the results:

foreach (var g in types)
{
    Debug.WriteLine(string.Format("{0} types where IsIDisposable is {1}", g.Types.Count(), g.IsIDisposable));
    if (g.IsIDisposable)
    {
        foreach (string t in g.Types)
        {
            Debug.WriteLine(t);
        }
    }
}

And find that my argument doesn't hold up.  Here are the results:

144 types where IsIDisposable is False
50 types where IsIDisposable is True

I left out the output that printed out all of the Context IDisposable types, as this list was fairly long.  I decided not to filter out non-public types, as MS tends to have lots of types marked "internal".  So it turned out that only 25% of types that end with "Context" implement IDisposable, so my assumptions were incorrect.

Other applications

LINQ provides a clean syntax to search assemblies using reflection.  I've also used it to argue against read-write properties for array types (they account for only 0.6% of all collection-type properties).  The continuations and joining lower the barrier for searching for specific types, properties, etc.  Since the LINQ methods are extension methods for any IEnumerable<T> type, you'll be pleasantly surprised when IntelliSense kicks in at the options available for querying and manipulating collections and arrays.

Tuesday, July 3, 2007

Limitations of generic base classes

I thought I had created something fairly useful with a generic Value Object in a previous post.  Generic base classes are nice, and there are several recommended base classes for creating collections classes.  Whenever I try to make an interesting API with a generic base class, limitations and faulty assumptions always reduce the usefulness of that base class.  Let's first start with a couple of classes that we'd like to include in a generic API:

public class Address
{
    private readonly string _address1;
    private readonly string _city;

    public Address(string address1, string city)
    {
        _address1 = address1;
        _city = city;
    }

    public string Address1
    {
        get { return _address1; }
    }

    public string City
    {
        get { return _city; }
    }
}

public class ExpandedAddress : Address
{
    private readonly string _address2;

    public ExpandedAddress(string address1, string address2, string city)
        : base(address1, city)
    {
        _address2 = address2;
    }

    public string Address2
    {
        get { return _address2; }
    }
}

Fairly basic classes, just two types of addresses, with one a subtype of the other.  So what kinds of issues do I usually run in to with generic base classes?  Let's look at a few different types of generic base classes to see.

Concrete generic implementations

A concrete generic implementation is a concrete class that inherits a generic base class:

public class AddressCollection : Collection<Address>
{
    public AddressCollection() {}

    public AddressCollection(IList<Address> list) : base(list) {}
}

Following the Framework Design Guidelines, I created a concrete collections class by inheriting from System.Collections.ObjectModel.Collection<T>.  I also have the ExpandedAddress class, so how do I create a specialized collection of ExpandedAddresses?  I have a few options:

  • Create an ExpandedAddressCollection class inheriting from AddressCollection
  • Create an ExpandedAddressCollection class inheriting from Collection<ExpandedAddress>
  • Use the existing AddressCollection class and put ExpandedAddress instances in it.

All of these seem reasonable, right?  Let's take a closer look.

Inherit from AddressCollection

Here's what the ExpandedAddressCollection class would look like:

public class ExpandedAddressCollection : AddressCollection
{
    public ExpandedAddressCollection() {}

    public ExpandedAddressCollection(IList<Address> list) : base(list) {}
}

That's not very interesting, it didn't add any information to the original AddressCollection.  What's more, this ExpandedAddressCollection ultimately inherits Collection<Address>, not Collection<ExpandedAddress>.  Everything I try to put in or get out will be an Address, not an ExpandedAddress.  For example, this code wouldn't compile:

List<ExpandedAddress> addresses = new List<ExpandedAddress>();
addresses.Add(new ExpandedAddress("Address1", "Austin", "TX"));

ExpandedAddressCollection addressList = new ExpandedAddressCollection(addresses);

Because of limitations in generic variance, namely that C# does not support generic covariance or contravariance.  Even though ExpandedAddress is a subtype of Address, and ExpandedAddress[] is a subtype of Address[], IList<ExpandedAddress> is not a subtype of IList<Address>.

Inherit from Collection<ExpandedAddress>

In this example, I'll just implement the ExpandedAddressCollection in the same manner as AddressCollection:

public class ExpandedAddressCollection : Collection<ExpandedAddress>
{
    public ExpandedAddressCollection() {}

    public ExpandedAddressCollection(IList<ExpandedAddress> list) : base(list) { }
}

Now I my collection is strongly types towards ExpandedAddresses, so the example I showed previously would now compile.  It seems like I'm on the right track, but I run into even more issues:

  • ExpandedAddressCollection is not a subtype of AddressCollection
  • Collection hierarchy does not match hierarchy of Addresses (one is a tree, the other is flat)
  • I can't pass an ExpandedAddressCollection into a method expecting an AddressCollection
  • Since there is no relationship between the two collection types, I can't use many patterns where a relationship is necessary

So even though my collection is strongly typed, it becomes severely limited in more interesting scenarios.

Use existing AddressCollection class

In this instance, I won't even create an ExpandedAddressCollection class.  Any time I need a collection of ExpandedAddresses, I'll use the AddressCollection class, and cast as necessary.  I won't be able to pass an IList<ExpandedAddress> to the constructor because of the variance issue, however.  If I need to include some custom logic in the collection class, I'll run into the same problems highlighted earlier if I'm forced to create a new subtype of AddressCollection.

So we've seen the limitations of concrete generic implementations, what other options do I have?

Constrained generic base classes

I'd like a way to propagate the concrete type parameter back up to the original Collection<T>.  What if I make the AddressCollection generic as well?  Here's what that would look like:

public class AddressCollection<T> : Collection<T>
    where T : Address
{
    public AddressCollection() {}

    public AddressCollection(IList<T> list) : base(list) {}
}

public class ExpandedAddressCollection : AddressCollection<ExpandedAddress>
{
    public ExpandedAddressCollection() {}

    public ExpandedAddressCollection(IList<ExpandedAddress> list) : base(list) { }
}

So now I have a constrained base class for an AddressCollection, and an implementation for an ExpandedAddressCollection.  What do I gain from this implementation?

  • ExpandedAddressCollection is completely optional, I could just define all usage through an AddressCollection<ExpandedAddress>
  • Any AddressCollection concrete type will be correctly strongly typed for an Address type

Again, with some more interesting usage, I start to run into some problems:

  • I can never reference only AddressCollection, as I always have to give it a type parameter.
  • Once I give it a type parameter, I run into the same variance issues as before, namely AddressCollection<ExpandedAddress> is not a subtype of AddressCollection<Address>
  • Since I can never define any method in terms of solely an AddressCollection, I either need to make the containing class generic or the method generic.

For example, I can write the following code:

public void TestGenerics()
{
    AddressCollection<Address> addresses = new AddressCollection<Address>();
    addresses.Add(new Address("132 Anywhere", "Austin"));

    int count = NumberOfAustinAddresses(addresses);

    AddressCollection<ExpandedAddress> expAddresses = new AddressCollection<ExpandedAddress>();
    expAddresses.Add(new ExpandedAddress("132 Anywhere", "Apt 123", "Austin"));

    count = NumberOfAustinAddresses(addresses);
}

private int NumberOfAustinAddresses<T>(AddressCollection<T> addresses)
    where T : Address
{
    int count = 0;
    foreach (T address in addresses)
    {
        if (address.City == "Austin")
            count++;
    }
    return count;
}

This isn't too bad for the implementation nor the client code.  I don't even need to specify a generic parameter in the method calls.  If I can live with generic methods, this pattern might work for most situations.

The only other problem I'll have is that I might need to create subtypes of AddressCollection, like I did above with ExpandedAddressCollection.  In this case, I'd can continue to make each subtype generic and constrained to the derived type:

public class ExpandedAddressCollection<T> : AddressCollection<T>
    where T : ExpandedAddress
{
    public ExpandedAddressCollection() {}

    public ExpandedAddressCollection(IList<T> list) : base(list) { }
}

Again, if I can live with generic methods, I would be happy with this implementation, as I now keep the same hierarchy as my Addresses.  It is a little strange declaring ExpandedAddressCollection with a type parameter (ExpandedAddressCollection<ExpandedAddress>), but I'll live.

There's one more type of generic base class I'd like to explore.

Self-constrained generic base classes

Sometimes, I need to implement a certain generic interface, such as IEquatable<T>.  I could simply pass in the concrete type into the generic parameter like this:

public abstract class ValueObject : IEquatable<ValueObject>

But if I'm trying to create an abstract or a base class for subtypes to use, I'll run into problems where derived types won't implement IEquatable<DerivedType>.  Instead, I can make this abstract class generic and self-constraining:

public abstract class ValueObject<T> : IEquatable<T>
    where T : ValueObject<T>

Now, derived types will implement IEquatable<DerivedType>, as I showed in my post on value types.  Unfortunately, subtypes will only implement the correct IEquatable<T> for the first derived class:

public class Address : ValueObject<Address>
public class ExpandedAddress : Address

In this case, ExpandedAddress is not a subtype of ValueObject<ExpandedAddress>, and therefore only implements IEquatable<Address>.  I can't use the same tricks with the constrained generic base class, as I would need to declare Address as generic, and therefore never instantiable by itself.  The self-constrained generic base or abstract class is unfortunately only useful in hierarchies one level deep.

Conclusion

So generics aren't the silver spoon I thought it would be, but there are some interesting proposals to allow variance for generic types out there.  I might not be able to cover all of the scenarios I'd like for a generic base class, but by identifying several options and their consequences, I can make a better decision on solving the problem.