What I look for in a Code Review

I recently put this bullet point list together for the team I’m currently working with.

Naming Conventions

• Naming conventions. We will follow Microsoft’s standard C# naming conventions. http://blogs.msdn.com/brada/articles/361363.aspx. Ignore the bit about putting using statements inside the namespace declaration.

General Principles

• The core imperative is to organise complexity.
• Clarity and readability is central. “Intention Revealing”
• Do not prematurely optimise for performance.
• Do not repeat yourself. Never copy-and-paste code.
• Decouple.
• Always try to leave the code you work on in a better state than before you started (the ‘boy scout’ principle)

Keep the source clean

• Always delete unused code. Including variables and using statements
• Don’t comment out code, delete it. We have source control to manage change.

Naming things

• The name should accurately describe what the thing does.
• Do not use shortenings, only use well understood abbreviations.
• If the name looks awkward, the code is probably awkward.

Namespaces

• Namespaces should match the project name + path inside the project. This is what VS will give you by default.
• Classes that together provide similar functions should be grouped in a single namespace.
• Avoid namespace dependency cycles.

Variables

• Use constants where possible. Avoid magic strings.
• Use readonly where possible
• Avoid many temporary variables.
• Never use a single variable for two different puposes.
• Keep scope as narrow as possible. (declaration close to use)

Methods

• The name should accurately describe what the method does.
• It should only do one thing.
• It should be small (more than 10 lines of code is questionable).
• The number of parameters should be small.
• Public methods should validate all parameters.
• Assert expectations and throw an appropriate error if invalid.
• Avoid deep nesting of loops and conditionals. (Cyclomatic complexity).

Classes

• The name should accurately describe what the class does.
• Classes typically represent data or services, be clear which your class is.
• Design your object oriented schema deliberately.
• A class should be small.
• A class should have one responsibility only.
• A class should have a clear contract.
• A class should be decoupled from its dependencies.
• Favour composition over inheritance.
• Avoid static classes and methods.
• Make the class immutable if possible.

Interfaces

• Rely on interfaces rather than concrete classes wherever possible.
• An interface is a contract for interaction.
• An interface should have a single purpose (ISP)

Tests

• All code should have unit tests if possible.
• Test code should have the same quality as production code.
• Write code test-first wherever possible.

Error Handling

• Only wrap code with a try..catch statement if you are expecting it to throw a specific exception.
• Unexpected errors should only be handled at process boundaries.
• Never ‘bury’ exceptions.

Mike Ormond interviews me about the MVC Framework

This week Mike Ormond made the trip down to Brighton to join me for lunch and talk about ASP.NET MVC. He’s put together a couple of videos based on our chat. The backdrop is Brighton’s most famous landmark, The Pavillion.

 

Multi-tenanted services with the Windsor WCF Facility

This is the sixth in a series of posts featuring the WCF Facility:

Windsor WCF Integration
WCF / Windsor Integration: Using the perWebRequest lifestyle
WCF / Windsor Integration: Adding Behaviours
Atom feeds with the Windsor WCF Facility
Windsor WCF Facility: MessageAction and MessageEnvelopeAction

Download the code for this post here:
http://static.mikehadlow.com/Suteki.Blog.zip

A while back I started writing a series of posts on Multi-tenancy. This is the idea that a single instance of your application can host multiple clients with varying requirements. An IoC container such as Windsor is an excellent enabling technology for doing this. It allows you to compose varying object graphs at runtime depending on some context.

For example, I recently had a client with an integration requirement to a legacy system that they had developed in house. The legacy system was deployed with one instance in London and another in New York. The US and UK requirements differed slightly and so two versions of the application had been developed. I wanted a single service that could handle the needs for both legacy systems. Since they were 90% similar it made sense to simply swap in different components for where they differed.

In this post I want to show how to host a single instance of a service that can compose different components depending on the host name.

Say we have two domain names: red.shop and blue.shop. We can configure IIS with the two host headers (bindings):

The first thing to notice when you do this with a standard WCF setup, is that you get an error:

“This collection already contains an address with scheme http.  There can be at most one address per scheme in this collection.” That’s right, WCF doesn’t play nicely with multiple host headers/bindings. This is a major complaint and Microsoft have gone some way to resolving it by providing a mechanism where you can specify a single address that service will listen for. Unfortunately that doesn’t help us. We want to listen for any request arriving at the service and then use its hostname to compose our components.

There is a workaround. The addresses for an endpoint are passed from IIS to WCF via the ServiceHostFactory. If you intercept this and pass an empty list of addresses, WCF falls back to using configured endpoint addresses.

All we need to do is write a custom ServiceHostFactory that grabs the addresses and then configures our service component with multiple endpoints. The WCF Facility already provides a custom ServiceHostFactory, the WindsorServiceHostFactory, so we can simply specialise that:

   1: using System;

   2: using System.Collections.Generic;

   3: using System.Diagnostics;

   4: using System.ServiceModel;

   5: using Castle.Facilities.WcfIntegration;

   6: using Castle.MicroKernel;

   7: using Suteki.Blog.Multitenanted.IoC;

   8:  

   9: namespace Suteki.Blog.Multitenanted.Wcf

  10: {

  11:     public class MultitenantedServiceHostFactory : WindsorServiceHostFactory<DefaultServiceModel>

  12:     {

  13:         public MultitenantedServiceHostFactory(){ }

  14:  

  15:         public MultitenantedServiceHostFactory(IKernel kernel)

  16:             : base(kernel)

  17:         { }

  18:  

  19:         public override ServiceHostBase CreateServiceHost(string constructorString, Uri[] baseAddresses)

  20:         {

  21:             AddEndpoints(constructorString, baseAddresses);

  22:  

  23:             // passing no baseAddresses forces WCF to use the endpoint address

  24:             return base.CreateServiceHost(constructorString, new Uri[0]);

  25:         }

  26:  

  27:         protected override ServiceHost CreateServiceHost(Type serviceType, Uri[] baseAddresses)

  28:         {

  29:             AddEndpoints(serviceType, baseAddresses);

  30:             return base.CreateServiceHost(serviceType, new Uri[0]);

  31:         }

  32:  

  33:         private static void AddEndpoints(Type serviceType, Uri[] baseAddresses)

  34:         {

  35:             var handler = ContainerBuilder.GlobalKernel.GetHandler(serviceType);

  36:             AddEnpoints(baseAddresses, handler);

  37:         }

  38:  

  39:         private static void AddEndpoints(string constructorString, Uri[] baseAddresses)

  40:         {

  41:             var handler = ContainerBuilder.GlobalKernel.GetHandler(constructorString);

  42:             AddEnpoints(baseAddresses, handler);

  43:         }

  44:  

  45:         private static void AddEnpoints(Uri[] baseAddresses, IHandler handler)

  46:         {

  47:             var endpoints = new List<IWcfEndpoint>();

  48:  

  49:             // create an endpoint for each base address

  50:             foreach (var uri in baseAddresses)

  51:             {

  52:                 endpoints.Add(WcfEndpoint.BoundTo(new BasicHttpBinding()).At(uri.ToString()));

  53:             }

  54:  

  55:             // add the endpoints to the service

  56:             handler.ComponentModel.CustomDependencies.Add(

  57:                 Guid.NewGuid().ToString(), 

  58:                 new DefaultServiceModel().Hosted().AddEndpoints(endpoints.ToArray()));

  59:         }

  60:     }

  61: }

All the action happens in the final AddEndpoints method. We use the WCF Facility’s fluent configuration to create an endpoint for each address that IIS gives us (effectively an address for each host header) and then add the endpoints to the Kernel’s component model for the service that is being hosted.

Note that I’m simply adding a custom dependency to the component model. The WCF Facility automatically searches the hosted component’s custom dependencies for service models. The DefaultServiceModel simply specifies the WCF BasicHttpBinding.

Note also that I have to use a service locator (ContainerBuilder.GlobalKernel) to get a reference to the Kernel because the WindsorServiceHostFactory’s kernel field is private rather than protected. It would be nice if this could be changed…. Craig?

Next we need to alter our svc file to point to our custom ServiceHostFactory:

<%@ ServiceHost Service="blogService" Factory="Suteki.Blog.Multitenanted.Wcf.MultitenantedServiceHostFactory, Suteki.Blog.Multitenanted"  %>

Now we can happily call the service from both http://red.shop/BlogService.svc and http://blue.shop/BlogService.svc.

If you simply want your service to work with multiple bindings this is all you have to do. However, as I described above, we might also want to compose our service based on the hostname.

For that we need to write an IHandlerSelector that can choose components based on the current host name. I’ve written about how to this in a web application here. Please read that first if you haven’t encountered IHandlerSelector before.  We use a similar approach, but this time we are going to use the very handy Windsor NamingPartsSubSystem to give our components a hostname parameter that we use to match on.

   1: using System;

   2: using System.Linq;

   3: using System.Diagnostics;

   4: using System.ServiceModel;

   5: using Castle.MicroKernel;

   6:  

   7: namespace Suteki.Blog.Multitenanted.IoC

   8: {

   9:     public class HostBasedComponentSelector : IHandlerSelector

  10:     {

  11:         private readonly IKernel kernel;

  12:  

  13:         public HostBasedComponentSelector(IKernel kernel)

  14:         {

  15:             this.kernel = kernel;

  16:         }

  17:  

  18:         public bool HasOpinionAbout(string key, Type service)

  19:         {

  20:             if (OperationContext.Current == null) return false;

  21:             

  22:             var componentKey = GetComponentKeyWithHostnameParameter(key, service);

  23:             return kernel.HasComponent(componentKey);

  24:         }

  25:  

  26:         private static string GetComponentKeyWithHostnameParameter(string key, Type service)

  27:         {

  28:             if (string.IsNullOrEmpty(key))

  29:             {

  30:                 key = service.Name;

  31:             }

  32:  

  33:             var hostname = OperationContext.Current.Channel.LocalAddress.Uri.Host;

  34:             return string.Format("{0}:host={1}", key, hostname);

  35:         }

  36:  

  37:         public IHandler SelectHandler(string key, Type service, IHandler[] handlers)

  38:         {

  39:             return handlers.First(h => h.ComponentModel.Name == GetComponentKeyWithHostnameParameter(key, service));

  40:         }

  41:     }

  42: }

Remember that the IHandlerSelector interface has two methods; HasOpinionAbout and SelectHandler. In HasOpinionAbout we combine the name of the service (or the service type whatever is provided) with the hostname and ask the kernel if it can provide a component. If it can, we supply that component from the SelectHandler method.

 

In our configuration we can specify any components that we want to be chosen by hostname:

 

   1: public static IWindsorContainer Build()

   2: {

   3:     var container = new WindsorContainer();

   4:     container.Kernel.AddSubSystem(SubSystemConstants.NamingKey, new NamingPartsSubSystem());

   5:  

   6:     var debug = new ServiceDebugBehavior

   7:     {

   8:         IncludeExceptionDetailInFaults = true

   9:     };

  10:  

  11:     container.AddFacility<WcfFacility>()

  12:         .Register(

  13:             Component.For<IServiceBehavior>().Instance(debug),

  14:             Component.For<ILogger>().ImplementedBy<DefaultLogger>().Named("ILogger:host=blue.shop"),

  15:             Component.For<ILogger>().ImplementedBy<AlternativeLogger>().Named("ILogger:host=red.shop"),

  16:             Component

  17:                 .For<IBlogService>()

  18:                 .ImplementedBy<DefaultBlogService>()

  19:                 .Named("blogService")

  20:                 .LifeStyle.Transient

  21:         );

  22:  

  23:     container.Kernel.AddHandlerSelector(new HostBasedComponentSelector(container.Kernel));

  24:     GlobalKernel = container.Kernel;

  25:     return container;

  26: }

Here we specify two different components for the ILogger service which our DefaultBlogService has a dependency on; DefaultLogger and AlternativeLogger. When we call the service at http://blue.shop/BlogService.svc we will get a response from the DefaultBlogService composed with the DefaultLogger, when we call http://red.shop/BlogService.svc it will be composed with AlternativeLogger.

 

Note that we add the NamingPartsSubSystem before doing any other configuration. The HostBasedComponentSelector can be added afterwards.

 

Now I can happily add new host headers and, so long as I recycle my app pool, my service will respond as expected. Nice!

Parameterised component resolution with the Windsor NamingPartsSubSystem

Here’s a neat little trick I’ve recently learnt that my favourite IoC container can do.

Sometimes you need to resolve different components for the same service depending on some property. This is easy to do in Windsor with the NamingPartsSubSystem.

First register the subsystem with the AddSubSystem method, passing in a new instance of the NamingPartsSubSystem.

You can then give your components names differentiated by any number of parameters. For example we can have several components with the base name ‘thing’ and with a comma separated list of properties, in our case colour and version. The syntax is:

<name>:<key 1>=<value 1>[,<key n>=<value n>]

We can now resolve components based on any combination of those parameters. If more than one component matches, the container simply returns the first one to be registered.

Here’s a little test to demonstrate:

   1: using Castle.MicroKernel;

   2: using Castle.MicroKernel.Registration;

   3: using Castle.MicroKernel.SubSystems.Naming;

   4: using Castle.Windsor;

   5: using NUnit.Framework;

   6:  

   7: namespace Suteki.Blog.Tests.Spikes

   8: {

   9:     [TestFixture]

  10:     public class NamingPartsSubsystemSpike

  11:     {

  12:         [SetUp]

  13:         public void SetUp()

  14:         {

  15:         }

  16:  

  17:         [Test]

  18:         public void GetComponentsByHostname()

  19:         {

  20:             var container = new WindsorContainer();

  21:             container.Kernel.AddSubSystem(SubSystemConstants.NamingKey, new NamingPartsSubSystem());

  22:  

  23:             container.Register(

  24:                     Component.For<IThing>().ImplementedBy<Thing1>().Named("thing:colour=red,version=1"),

  25:                     Component.For<IThing>().ImplementedBy<Thing2>().Named("thing:colour=blue,version=1"),

  26:                     Component.For<IThing>().ImplementedBy<Thing3>().Named("thing:colour=red,version=2"),

  27:                     Component.For<IThing>().ImplementedBy<Thing4>().Named("thing:colour=blue,version=2")

  28:                 );

  29:  

  30:             var defaultThing = container.Resolve("thing");

  31:  

  32:             Assert.That(defaultThing, Is.Not.Null);

  33:             Assert.That(defaultThing, Is.TypeOf(typeof(Thing1)));

  34:  

  35:             var redThing = container.Resolve("thing:colour=red");

  36:  

  37:             Assert.That(redThing, Is.Not.Null);

  38:             Assert.That(redThing, Is.TypeOf(typeof(Thing1)));

  39:  

  40:             var blueThing = container.Resolve("thing:colour=blue");

  41:  

  42:             Assert.That(blueThing, Is.Not.Null);

  43:             Assert.That(blueThing, Is.TypeOf(typeof(Thing2)));

  44:  

  45:             var version2 = container.Resolve("thing:version=2");

  46:  

  47:             Assert.That(version2, Is.Not.Null);

  48:             Assert.That(version2, Is.TypeOf(typeof(Thing3)));

  49:  

  50:             var blueThingVersion2 = container.Resolve("thing:colour=blue,version=2");

  51:  

  52:             Assert.That(blueThingVersion2, Is.Not.Null);

  53:             Assert.That(blueThingVersion2, Is.TypeOf(typeof(Thing4)));

  54:         }

  55:  

  56:         private class IThing {}

  57:         private class Thing1 : IThing {}

  58:         private class Thing2 : IThing {}

  59:         private class Thing3 : IThing {}

  60:         private class Thing4 : IThing {}

  61:     }

  62: }