Shame On Me

Someone decided to publicly criticize my example code in my previous post by saying that it was “bad.”  While I don’t agree with the nature of the criticism (my blog *does* have comments, you know) I totally agree with the message.  It was bad code.  I would never place that in a production application.

The intent of the post was to demonstrate some ajax techniques, not to talk about how to write testable, decoupled controllers.  I suppose that example code should come with a bit of a disclaimer that says “hey, we are doing this here for simplicity’s sake, but obviously you’d abstract this out.”  Ideally I would have commented out the entire flickr section, because it’s totally not important in the context of the example.

In light of this topic, here is how a perfectly testable PhotosController might be written:

public class PhotosController : Controller
{        
    private readonly IPhotoRepository _photoRepository;

    public PhotosController(IPhotoRepository photoRepository)
    {
        _photoRepository = photoRepository;
    }

    [ControllerAction]
    public void Search()
    {            
        RenderView("Search", new Photo[] { });
    }        

    [ControllerAction]
    public void Find(string query, bool? ajax)
    { 
        
        var photos = _photoRepository.SearchPhotos(query);
          
        ViewData["query"] = query;
        ViewData["photos"] = photos;
        
        if(ajax == true)
        {
            RenderView("_images", photos);
        }
        else
        {
            RenderView("results");    
        }            
    }
}

Here you can clearly see our dependency on IPhotoRepository.  This guy can be anything we want, from a Flickr implementation, google images implementation, a file system directory, or even (gasp) a TEST implementation for writing unit tests!

Here is the IPhotoRepository, that we are using in the example:

public interface IPhotoRepository
{
    Photo[] SearchPhotos(string query);
asdf
}

This returns a Photo object (that I defined), which means whatever implementation you choose, you need to map to this object. This is a good thing because it removed my dependency on "FlickrPhoto" in the view itself.

Our FlickrPhotoRepository now looks like this:

public class FlickrPhotoRepository : IPhotoRepository
{
    private readonly IConfigSource _ConfigSource;

    public FlickrPhotoRepository(IConfigSource configSource)
    {
        _ConfigSource = configSource;
    }

    public Photo[] SearchPhotos(string query)
    {
        var flickr = new Flickr(_ConfigSource.GetSetting("flickr.api.key"));
        var results = flickr.PhotosSearch(query, TagMode.AnyTag, query, 40, 1);

        var photos = new List<Photo>();
        foreach (var item in results.PhotoCollection)
        {
            photos.Add(new Photo(item.SquareThumbnailUrl, item.Title, item.PhotoId));
        }

        return photos.ToArray();
    }
}

Wait a minute! This class now has it's own dependency on IConfigSource? Craziness! In our example code from before, I had accessed the Configuration singleton directly, which is punishable by hand-removal in Albonia. Thank goodness I don't live there. Now this class is decoupled from the actual configuration implementation and it could really be coming from a text file or database (whatever you want).

Wiring all this up is easy.  You could do it via XML, but I chose to do it directly in code:

private void InitializeWindsor()
{
    _container = new WindsorContainer();
    _container.AddComponent("config-source", typeof(IConfigSource), typeof(AppDomainConfigSource));        
    foreach(Type type in Assembly.GetExecutingAssembly().GetTypes())
    {             
        if(type.IsSubclassOf(typeof(Controller)))
        {
            _container.AddComponent(type.Name, type);
        }
    }
}

Also note that I'm using WindsorControllerFactory from MVCContrib, so now my controller can be created (and each of it's dependencies ( and their dependencies (and their Dependencies)))... I think you get the idea.

And there you have it.  A perfectly decoupled and testable modification for my previous example code.  As you were….