== Things I’d change Testing wise there are some tests covering the MetaWeblogService class although it feels like the setup in each of the tests is a bit heavy. I’d refactor the tests using compose method to try and remove some of the noise and rename the tests so that they follow the BDD style more closely. When time is used in the code it is done by directly using the DateTime class. Testing time based code is made much easier when we can control the time in our tests, so have a Time Provider or System Clock class can be a very useful approach here. A classicist approach has been taken towards testing with no mocks in sight! It feels like a lot of work went into creating the fake versions of the classes which could maybe have been done much more easily using a stub in Rhino Mocks for example. A lot of the ViewData code is weakly typed by putting values into an array. We started with this approach on a project I worked on and it’s so easy to misspell something somewhere that we came to the conclusion that if we can get strong typing we should strive for that. Jeremy Miller’s Thunderdome Principle works well here. AdminController seems to have a massive number of responsibilities which is perhaps not surprising given the name. I think it’d be better to have the administrative tasks handled from the individual controllers for those tasks. This would also help lead the design towards a more RESTful approach The Domain model is very driven from the definition of the database tables from my understanding. The domain objects are effectively representations of database tables. If we want to create a richer domain model then it would make more sense to design the classes in terms of what makes sense in the domain rather than what makes sense at a database level. An ORM tool could then be used to map the database tables to the code. A lot of the domain objects implement an interface as well which I don’t think is really necessary. Although there are exceptions, in general when we refer to domain objects we care about the implementation rather than a contract describing what it does. The Routes collection is passed around quite a few of the controllers, seemingly so that URLs can be generated inside other pages. It seems a bit overkill to pass this around to achieve such a simple goal and my thinking is that maybe a wrapper class which generated the URLs might be more intention revealing. ** There is a bit more logic than I’m comfortable with in some of the views - I think it would be good to move this logic into ViewModel classes. This will have the added benefit of allowing that logic to be tested more easily.

+ == Want to know more about I’m curious as to why LINQ to SQL is being used as the interface to the database as I was under the impression that it is being phased out by Microsoft. The syntax seemed quite readable but I think the problem of interacting between code and the database in a clean way has been largely solved by NHibernate although the Entity Framework is a newish addition in this area. One interesting thing I noticed was a lot of Background Services running in this codebase - I’ve not come across this in a web application before. They are actually being used for creating trackbacks, sending trackbacks and sending email messages.

+ == My learning from reading the code I asked for some advice on the best way to read code on Twitter and the most popular advice was to debug through the code. Unfortunately I couldn’t seem to do this without having a database in place so another approach was necessary. Instead I started reading from the tests that were available and then clicked through to areas of interest from there. I think it worked reasonably well but it wasn’t as focused as if I had debugged and I couldn’t see the state of the program as it executed. I wanted to find a way to read the Oxite code and navigate to areas of the ASP.NET MVC source using Reflector without having to do so manually. TestDriven.NET was recommended to me and it worked really well. Clicking the 'Go to Reflector' option from the menu takes you to the current class in the Reflector window. Impressive. Changing the Resharper find usages menu to show 'Namespace + Type' makes it much easier to try and work out what the code is doing rather than the default setting of just 'Namespace'. From looking at some of the ASP.NET MVC code I realised that a lot of data is stored in static variables in order to make the data globally accessible. It’s something I had never considered this before and it makes sense in a way but feels a little nasty * I found that I was getting *side tracked quite a lot by irrelevant details in the code. I’m used to having a pair guide me through a new code base so looking at this one alone was a bit different for me. Separating noise/signal when reading code and identifying common patterns to allow me to do this is something I am working on.

== Overall

+ I think it’s really cool that the Oxite team put their code out there for people to look at and learn from. A number of highly experienced developers have made suggestions for improvement so clearly this is quite a useful way to get feedback and code better in the future. From what I understand, Rob Conery is working on some refactorings for this code base so it will be interesting to see what it looks like when this is done.