Thursday 26 March 2009

TDD with EPiServer

During the last Demo Day (conference organized by Cognifide every 3-4 months) I had a chance to talk about Test Driven Development in EPiServer projects. In this post I would like to recap main points of my presentation.

Obstacles on the way to TDD

Unfortunately it's very hard to apply TDD to ASP.NET WebForms as it is. WebForms was designed as a platform for Rapid Application Development. Features like wide set of rich controls and state management (view state) can significantly speed up development but in the same time WebForms does not provide decent separation of concerns. I think code-behind files were suppose to give us some separation between the HTML and application logic. And to be fair it's "some" separation, but we still need to deal in one code-behind file with ASP.NET components, which are tightly bound to the underlying HttpContext, presentation logic and usually a few other things. It's clearly too much. There are way too many concerns in one place, too many dependencies on ASP.NET runtime. It makes code in applications like this hard to maintain and also makes writing unit test impossible.

Another thing with which you have to cope with is EPiServer. To get an idea of problems which you may encounter check this unit tests:

   1:  [Test]
   2:  public void MyEPiServerUnitTest()
   3:  {
   4:      var page = new PageData();
   5:      page.Property.Add("MetaAuthor", new PropertyString { Value = "MetaAuthor123" });
   6:      page.Property.Add("Heading", new PropertyString { Value = "Heading123" });
   7:      page.Property.Add("MainBody", new PropertyXhtmlString { Value = "MainBody123" });
   8:  
   9:      Assert.IsTrue(page["MainBody"].Equals("MainBody123"));
  10:  }

Basically I'm creating PageData instance with a few properties and then I'm checking if correct values will be returned. This is probably the simplest unit test ever. So if you run it then you don't expect anything else then a green bar right? Surprisingly, instead of green bar you will get exception saying:

   System.ApplicationException: First time you call Settings.Instance you must have a valid HttpContext.

If you want to know who you should blame for calling Settings.Instance then check PropertyXhtmlString :)

As you may know, HttpContext can be mocked, it's not a problem. But in our case it won't help much, we won't get this exception of course, but we will get different one instead.

As you can see the way to TDD is not easy, EPiServer is also closely bound to HttpContext.

Deal with problems in two steps

Step 1 - Isolate as much functionality as you can away from ASP.NET runtime. How? Introduce design pattern called Model-View-Presenter (MVP). This pattern divides the responsibilities into separate classes:
  • View - those are *.aspx and *.ascx files including code-behind files. This layer should be as thin as possible, code-behind files should delegate as much functionality as possible to the Presenter.
  • Presenter is responsible for interface logic which includes initialization of view and dealing with all events. It's important that presenter shouldn't have any reference to System.Web namespace and should be separated from the concrete view with an interface. Additionally presenter has access to business layer and/or backend services.
  • Model - the data on which application is working on, for EPiServer those might be pages, users, files, data stored in the HttpSession etc.
It's worth mentioning that in general there are two variations of MVP design pattern:

There are minor differences between both versions, you can learn more about both versions on Martin Fowler's site: Passive View and Supervising Controller.

Let me show you how it works in practice. I'm going to implement well known news page from EPiServer's public templates, the page for visitors looks like this:


Areas which are set by news page are marked with red rectangles. Other data comes from MasterPage. Below you can check how the aspx file:

   1:  <asp:Content ID="Content4" ContentPlaceHolderID="MainBodyRegion" runat="server">
   2:      <div id="MainBody">
   3:          <h1><asp:Literal ID="litHeading" runat="server" /></h1>
   4:          <asp:Literal ID="litMainBody" runat="server" />
   5:      </div>    
   6:  </asp:Content>
   7:  
   8:  
   9:  <asp:Content ID="Content5" ContentPlaceHolderID="SecondaryBodyRegion" runat="server">
  10:   <div id="SecondaryBody">
  11:      <dl>
  12:          <dt><EPiServer:Translate ID="author" runat="server" Text="/news/writername" /></dt>
  13:          <dd><asp:Literal ID="litMetaAuthor" runat="server" /></dd>
  14:          <dt><EPiServer:Translate ID="Translate1" runat="server" Text="/news/publishdate" /></dt>
  15:          <dd><asp:Literal ID="litPageStartPublish" runat="server" /></dd>
  16:      </dl>
  17:  </div>
  18:  </asp:Content>

As you can see, so far nothing surprising. But now, instead of jumping to the code-behind file, you should create an interface for the view. I'm going to call my interface INewsView and this is how it looks like:

   1:  public interface INewsView
   2:  {
   3:      String Heading { get; set; }
   4:      String MainBody { get; set; }
   5:      String MetaAuthor { get; set; }
   6:      String PageStartPublish { get; set; }
   7:  }

INewsView interface defines a contract between presenter and view. View needs to get above data from presenter and presenter is obligated to deliver them.

Having the interface ready we can start working on the presenter, the simplest implementation looks like this:

   1:  public class NewsPresenter : AbstractPresenterBase<INewsView>
   2:  {
   3:      public override void Initialize()
   4:      {
   5:          View.MetaAuthor = PresenterUtils.GetPropertyValue(CurrentPage, "MetaAuthor", "");
   6:          View.PageStartPublish = PresenterUtils.GetProperty<DateTime>(CurrentPage, "PageStartPublish", "",
   7:                                                                       x => x.ToShortDateString());
   8:  
   9:          View.Heading = PresenterUtils.GetPropertyValue(CurrentPage, "Heading", "");
  10:          View.MainBody = PresenterUtils.GetPropertyValue(CurrentPage, "MainBody", "");
  11:      }
  12:  }

PresenterUtils class is just my helper which is returning values of properties if they are set, or default values otherwise. There is no magic here but as you may notice, logic dealing with CurrentPage is located in one place which makes it much more reusable.

Now we have NewsPresenter ready we can piece it all together. The only missing part is implementation of INewsView interface:


   1:  public partial class NewsItem : SimpleMVPPageBase<NewsPresenter, INewsView>, INewsView
   2:  {
   3:      protected override INewsView ViewImplementation
   4:      {
   5:          get { return this; }
   6:      }
   7:  
   8:      public String MetaAuthor
   9:      {
  10:          get { return litMetaAuthor.Text; }
  11:          set { litMetaAuthor.Text = value; }
  12:      }
  13:  
  14:      public String PageStartPublish
  15:      {
  16:          get { return litPageStartPublish.Text; }
  17:          set { litPageStartPublish.Text = value; }
  18:      }
  19:  
  20:      public string Heading
  21:      {
  22:          get { return litHeading.Text; }
  23:          set { litHeading.Text = value; }
  24:      }
  25:  
  26:      public string MainBody
  27:      {
  28:          get { return litMainBody.Text; }
  29:          set { litMainBody.Text = value; }
  30:      }
  31:  }

This class is located in code-behind file, it implements INewsView interface and represents a link between NewsPresenter and the view and actual ASP.NET controls.

What you can't see in this class is how the presenter is instantiated, when the view implementation is passed to the presenter, this is wrapped in SimpleMVPPageBase class:

   1:  public abstract class SimpleMVPPageBase<PresenterClass, ViewInterface> : SimplePage
   2:      where PresenterClass : AbstractPresenterBase<ViewInterface>, new()
   3:  {
   4:      protected PresenterClass Presenter;
   5:      protected abstract ViewInterface ViewImplementation { get; }
   6:  
   7:      protected override sealed void OnInit(EventArgs e)
   8:      {
   9:          base.OnInit(e);
  10:          Presenter = new PresenterClass
  11:          {
  12:              View = ViewImplementation,
  13:              CurrentPage = CurrentPage
  14:          };
  15:      }
  16:  
  17:      protected override sealed void OnLoad(EventArgs e)
  18:      {
  19:          base.OnLoad(e);
  20:          if (!IsPostBack)
  21:          {
  22:              Presenter.Initialize();
  23:              DataBind();
  24:          }
  25:      }        
  26:  }

SimpleMVPPageBase class is responsible for two things:
  • It instantiates presenter class, and sets for it CurrentPage and view implementation.
  • Also, within OnLoad, it calls Presenter.Initialize() method which should initialize the view.
With all those pieces in place, you should get a working page. So, in fact we are in the starting position but with this version we have significantly better separation of concerns and our logic is testable:


   1:  public class Checking_news_view_initialization : AbstractPresenterTest<NewsPresenter>
   2:  {
   3:      protected override void Before_each_test()
   4:      {
   5:          Presenter.CurrentPage = EPiServerPage.CreateBlankPage()
   6:              .AddProperty<PropertyString>("MetaAuthor", "author123")
   7:              .AddProperty<PropertyString>("Heading", "heading123")
   8:              .AddProperty<PropertyXhtmlString>("MainBody", "mainBody123")
   9:              .AddProperty<PropertyDate>(EPiProperties.PageStartPublish, DateTime.Today)
  10:              .Instance();
  11:  
  12:          Presenter.View = mocks.StrictMock<INewsView>(null);
  13:      }
  14:  
  15:      [Test]
  16:      public void It_should_set_all_properties()
  17:      {
  18:          using (mocks.Record())
  19:          {
  20:              Expect.Call(Presenter.View.Heading).SetPropertyWithArgument("heading123");
  21:              Expect.Call(Presenter.View.MainBody).SetPropertyWithArgument("mainBody123");
  22:              Expect.Call(Presenter.View.MetaAuthor).SetPropertyWithArgument("author123");
  23:              Expect.Call(Presenter.View.PageStartPublish).SetPropertyWithArgument(DateTime.Today.ToShortDateString());
  24:          }
  25:  
  26:          using (mocks.Playback())
  27:          {
  28:              Presenter.Initialize();
  29:          }
  30:      }
  31:  }


Within Before_each_test() method I'm basically creating an instance of PageData with a few properties. I'm also mocking the view using Rhino Mocks. Thanks to that in It_should_set_all_properties() method I can record my expectations and later verify them.

But wait a second ... as I was writing earlier, EPiServer requires valid HttpContext right?

Workaround for this issue is a step 2 on our way to TDD:

   1:  [TestFixture]
   2:  public abstract class AbstractPresenterTest<PresenterClass> where PresenterClass : new()
   3:  {
   4:      protected PresenterClass Presenter { get; private set; }
   5:      protected MockRepository mocks { get; private set; }
   6:  
   7:  
   8:      protected AbstractPresenterTest()
   9:      {
  10:          Presenter = new PresenterClass();
  11:      }
  12:  
  13:      [TestFixtureSetUp]
  14:      protected void SetUpContextForAWholeFixture()
  15:      {
  16:          var settings = new EPiServer.Configuration.Settings
  17:                             {
  18:                                 StringCompressionThreshold = 0
  19:                             };
  20:  
  21:          Type settingsType = settings.GetType();
  22:          settingsType.GetField("_instance", BindingFlags.Static | BindingFlags.NonPublic)
  23:              .SetValue(settings, settings);
  24:      }
  25:  
  26:      [SetUp]
  27:      protected void SetUpContextForEachTest()
  28:      {
  29:          mocks = new MockRepository();
  30:          Before_each_test();
  31:      }
  32:  
  33:      protected abstract void Before_each_test();
  34:  }


HttpContext related exception won't be triggered if you create instance of Settings and using reflection assign it to the static and private field _instance on Settings class. This way you can also provide your configuration for tests.

Conclusions
  • the most important one is that Test Driven Development with EPiServer is possible!
  • you need to isolate as much functionality from ASP.NET runtime and MVP design patter is doing exactly that for you
  • with a little bit of hacking of EPiServer you can instantiate PageData and test your logic
  • MVP requires additional effort to create interfaces, presenters etc. Downside is that you won't be able to build web applications so rapidly but it should pay off in longer and more complex projects where having a set of automated tests is a big advantage.
Credits
  • My research and presentation were inspired by Raymond Lewallen who was a guest of Poznan .Net Group and was presenting Behavioral Driven Development approach.
  • I have used image from Microsoft site talking about MVP.

Wednesday 11 March 2009

Validation of NHibernate Entities

Recently I was reading Billy McCafferty's post "A Few NHibernate Tips" and one point there, related to mapping files, was particularly interesting for me:
Don’t bother including database meta data (e.g., column length) in mapping files unless intending to auto generate the SQL using the hbm2dll tool.
I was really surprised to see that things like column length, information if column accept null values are completely ignored.

Why does it matter?

There are actually two reasons:
  • if details regarding database meta data are irrelevant then why should we write mappings at all? Fluent Nhibernate and Auto Mapping is the way to go.
  • performance impact - it's a huge waste of time to make a call to database just to get an exception saying that some columns don't accept nulls. Luckily, there is a project called NHibernate Validator which can solve this issue.
Fluent NHibernate and NHibernate Validator in one project

It's fairly simple to configure those two to work together:

   1:  public static ISessionFactory GetFluentlyConfiguredSessionFactory()
   2:  {
   3:      return Fluently.Configure()
   4:          .Database(MsSqlConfiguration
   5:                        .MsSql2005
   6:                        .ConnectionString(c => c.Is(ConnectionString)))
   7:          .Mappings(m =>
   8:                    m.FluentMappings.AddFromAssemblyOf<Product>()
   9:                        .ConventionDiscovery.Add(new AdventureWorksConvention())
  10:                        .ExportTo(ExternalMappingsOutputDirectory))
  11:          .ExposeConfiguration(ConfigureNHibernateValidator)
  12:          .BuildSessionFactory();
  13:  }
  14:  
  15:  public static void ConfigureNHibernateValidator(Configuration configuration)
  16:  {
  17:      var nhvc = new NHVConfiguration();
  18:      nhvc.Properties[NHibernate.Validator.Cfg.Environment.ValidatorMode] = "UseAttribute";
  19:      nhvc.Mappings.Add(new NHibernate.Validator.Cfg.MappingConfiguration("AdventureWorksPlayground", null));
  20:      
  21:      var validator = new ValidatorEngine();
  22:      validator.Configure(nhvc);
  23:  
  24:      //Registering of Listeners and DDL-applying here
  25:      ValidatorInitializer.Initialize(configuration, validator);
  26:  }

First method - GetFluentlyConfiguredSessionFactory() is responsible for fluent-nh configuration. Second method, ConfigureNHibernateValidator(), instantiates Validator and registers listeners.
NHibernate Validator has two built-in NHibernate event listeners. Whenever a PreInsertEvent or PreUpdateEvent occurs, the listeners will verify all constraints of the entity instance and throw an exception if any of them are violated. Basically, objects will be checked before any insert and before any update triggered by NHibernate. This includes cascading changes! This is the most convenient and easiest way to activate the validation process. If a constraint is violated, the event will raise a runtime InvalidStateException which contains an array of InvalidValues describing each failure.
In your project you would like to change string "AdventureWorksPlayground", that is a name of your assembly. Probably you don't want to change validation mode which is set to "UseAttribute". Alternative is that you can use XML file for it.

Here is an example of domain object with a few validation rules:

   1:  public class SalesTerritory
   2:  {
   3:      public virtual int Id { get; set; }
   4:      
   5:      [NotNull]
   6:      [Pattern(Regex = "[A-Za-z0-9]+")]
   7:      public virtual string Name { get; set; }
   8:  
   9:      [NotNull]
  10:      public virtual string Group { get; set; }
  11:      
  12:      [Length(Max = 3), NotNull]
  13:      public virtual string CountryRegionCode { get; set; }
  14:  }

Out of the box you can use number different attributes to describe your validation rules, you can also create your own one if necessary. I was running a few tests and the results show that Validator will throw an exception four times faster then the NHibernate itself (and that was with database on the same machine).

You can learn much more about NHibernate Validator on NHibernate Forge wiki.

Shout it

Monday 9 March 2009

Fluent NHibernate and Inheritance Mapping

While exploring the AdventureWorks database I have found an interesting case where inheritance mapping has to be used. In this post I would like to show how it can be neatly mapped with Fluent NHibernate. Lets start with database schema:

There are a few interesting things about this few tables:
  • There are two types of customers: Store and Individual
  • Each customer is represented as a single record in Customer table and additionally single record in Individual or Store table. Customer has a CustomerType column based on which we can determine the type.
  • Individual and Store tables have a primary key which is also a foreign key to the main Customer table. It means that Customer with ID 1231 will have corresponding record in Store (or Individual) table with the same ID.
Why inheritance is needed here?

The above tables can be also mapped without any fancy inheritance so question is why should we bother? The real reason for this hassle are business rules which are different for each type of customer (and potentially sales territory). To be more specific, there might be different rules for queuing deliveries, calculating discounts etc.

It makes sense to encapsulate those rules in a separate class for each type of consumer. This line of thinking leads us to the class diagram like this:


Customer is marked as a abstract class so it can't be instantiated. For simplicity I have added here only one method GetCurrentDiscount() which represents business logic specific for each type. Additionally StoreConsumer and IndividualConsumer classes have a Details property which "links" to two other tables - Store or Individual.

Inheritance Mapping

In general NHibernate supports three different inheritance mapping strategies. In this case I will use table per class hierarchy strategy. With Fluent NHibernate Customer class can be mapped like this:

   1:  public class CustomerMap : ClassMap<Customer>
   2:  {
   3:      public CustomerMap()
   4:      {
   5:          Id(x => x.Id).GeneratedBy.Native();
   6:  
   7:          Map(x => x.ModifiedDate).Not.Nullable();
   8:          Map(x => x.AccountNumber).ReadOnly().Unique();
   9:  
  10:          References(x => x.SalesTerritory, "TerritoryID");
  11:          
  12:          DiscriminateSubClassesOnColumn<string>("CustomerType")
  13:              .SubClass<IndividualCustomer>("I",
  14:                                            m => m.HasOne(x => x.Details)
  15:                                                     .Cascade.SaveUpdate()
  16:                                                     .FetchType.Join())
  17:              .SubClass<StoreCustomer>("S",
  18:                                       m => m.HasOne(x => x.Details)
  19:                                                .Cascade.SaveUpdate()
  20:                                                .FetchType.Join());
  21:      }
  22:  }
Let me explain what is going on up there:
  • Thanks to Id(...) and Map(...) methods we can tell Fluent NHibernate about identity column and other simple "data" columns. You can find more about basics here.
  • References(...) method is used to map many-to-one association with SalesTerritory table. In this case column name doesn't follow the convention therefore it has to be explicitly specified.
  • CustomerType column is used as a discriminator. "The discriminator column contains marker values that tell the persistence layer what subclass to instantiate for a particular row." CustomerType can have two values:
    • I for individual customer, in which case IndividualCustomer class will be instantiated or
    • S for store and for StoreCustomer class.
  • Both subclasses map one additional one-to-one association. Based on above mapping you can't see that actually each subclass "links" to the different table, it will be clear though when you check both classes:
   1:  public class IndividualCustomer : Customer
   2:  {
   3:      public virtual Individual Details { get; set; }
   4:      
   5:      public override double GetCurrentDiscount()
   6:      {
   7:          // some logic here
   8:      }
   9:  }

And StoreCustomer:

   1:  public class StoreCustomer : Customer
   2:  {
   3:      public virtual Store Details { get; set; }
   4:      
   5:      public override double GetCurrentDiscount()
   6:      {
   7:          // some logic here
   8:      }
   9:  }

Please note that Details property has different type in each case, in each case NHibernate will be able to figure out which class should be created and what data should be populated ... awesome!

One-to-one association

There is one additional feature worth mentioning - as it was stated earlier, ID from Customer table equals ID from Store (or Individual) table. How it can be mapped to make sure that inserts will work? I will explain it based on Individual class:

   1:  public class Individual
   2:  {
   3:      public virtual int Id { get; set; }
   4:      public virtual int Contact { get; set; }
   5:      public virtual string Demographics { get; set; }
   6:      public virtual DateTime ModifiedDate { get; set; }
   7:      public virtual IndividualCustomer Customer { get; set; }
   8:  }
Individual class is always associated with individual customer therefore we can explicitly say that property Customer is of type IndividualCustomer. Other properties are fairly straightforward.

Here is the mapping class:

   1:  public class InidividualMap : ClassMap<Individual>
   2:  {
   3:      public InidividualMap()
   4:      {
   5:          Id(x => x.Id, "CustomerID").GeneratedBy.Foreign("Customer");
   6:  
   7:          Map(x => x.Demographics);
   8:          Map(x => x.ModifiedDate);
   9:          Map(x => x.Contact, "ContactID");
  10:  
  11:          HasOne(x => x.Customer).Constrained();
  12:      }
  13:  }

Appropriate generator for identity column is the key here:
foreign - uses the identifier of another associated object. Usually used in conjunction with a one-to-one primary key association.
With above mapping foreign generator knows about the Customer and will use exactly the same ID. More about foreign generator and different types of one-to-one association you can find here.

Related articles:

Shout it

Thursday 5 March 2009

EPiServer Search with VirtualPathVersioningProvider

EPiServer out of the box provides versioning file system handled by VirtualPathVersioningProvider. In this post I would like to show you step by step how to enable search for this provider.

Step 1 - Make sure that you are using VirtualPathVersioningProvider.

It sounds funny but it's worth double checking if your website is configured to use versioning file system or native file system (VirtualPathNativeProvider).

The best way is to check web.config section in which virtual providers are defined. You should find there lines like this:

   1:  <add showInFileManager="true" 
   2:       virtualName="Documents" 
   3:       virtualPath="~/Documents/" 
   4:       bypassAccessCheck="false" 
   5:       maxVersions="5" 
   6:       name="SiteDocuments"
   7:       type="EPiServer.Web.Hosting.VirtualPathVersioningProvider,EPiServer"
   8:       physicalPath="C:\EPiServer\VPP\ExampleEPiServerSite\Documents">

physicalPath property specifies where actually files will be stored. Interesting is that the paths and the filenames are stored in a database, only content is located in the file system. Internally, VirtualPathVersioningProvider keeps content in a fairly characteristic way:


Step 2 - EPiServer Indexing Service configuration


Indexing Service configuration is saved in a EPiServer.IndexingService.exe.config file which can be found in the same folder where the EPiServer Indexing Service was installed.

To let know indexing service about your folders you need to alter following part of configuration:

   1:  <episerver.indexingService>
   2:    <indexes>
   3:      <add connectionString="Data Source=server_name;Database=db_name;User Id=username;Password=password;Network Library=DBMSSOCN;" 
   4:           databaseClient="" 
   5:           filePath="C:\EPiServer\VPP\ExampleEPiServerSite\Documents" 
   6:           itemRoot="/Documents" />
   7:    </indexes>
   8:  </episerver.indexingService>

Two things are important here:
  • you need to use the same database here (connectionString) which your application is using
  • and the same path (filePath)

Step 3 - Make sure that Indexing Service works

The best way to makes sure that your folder was indexed is to check if folder called index was created. (Check the first image) This folder contains indexing service's data.

If your service is started and index folder is not present then probably good idea is to check indexing service logs, usually you can find there helpful information.

Lack of index folder can also cause error like this:

VirtualPathProvider 'SiteDocuments', Search Error: C:\EPiServer\VPP\ExampleEPiServerSite\Documents\index not a directory

Step 4 - Check if it works Probably the fastest way to check if search works it to use EPiServer's build-in file management tool:


Then you can access search feature:


I hope that this post will save a few people a bit of time. If you have been working with EPiServer search before and have interesting experiences then leave a comment, your input is valuable!

Tuesday 3 March 2009

DefaultButton - Deal with users hitting ENTER on your forms

Developers tend to assume that users will be always clicking on the buttons to submit forms. Is it a valid assumption? Unfortunately not always ... the simplest example can be quick search, a common component on many sites.


What if user simply hit ENTER instead of clicking on the button next to the text box? Because our button wasn't clicked then of course even handler for Click event won't be invoked. The problem is that users expect that hitting Enter will correctly submit the form. So what can we do?

HtmlForm and Panel classes have a property called DefaultButton. Thanks to this property we can address the issue. First lets check the HtmlForm.DefaultButton property, here is how it can be used:

   1:  protected void Page_Load(object sender, EventArgs e)
   2:  {
   3:      Page.Form.DefaultButton = btnSearch.UniqueID;
   4:  }

That's all what is needed to make sure that post back generated by pressing the ENTER key will be "associated" with the button btnSearch. That is an improvement, but there is another problem to solve, check contact us page:

This is slightly more complex situation as here we would like to have Send button to be the default button for HtmlForm. So what about quick search? The following piece of code will solve it:

   1:  <asp:panel id="pnlSearch" runat="server" defaultbutton="btnSearchButton">
   2:      <asp:textbox id="txbSearchQuery" runat="server" validationgroup="search" maxlength="50" />
   3:      <asp:Button id="btnSearchButton" runat="server" text="Search" validationgroup="search" />
   4:  </asp:panel>

By surrounding the quick search controls with ASP.NET panel we can define for that group default button, different default button then the one on HtmlForm. In this simple way it's possible to define arbitrary number of small areas on the page with custom default button.

Another thing worth noticing is that defining unique validation group for all functionally related groups of controls is considered as a good practice. This way you can be sure that submitting one group won't trigger validation somewhere else.

I know that those are not a life-changing features but still I find them very useful, it's good to dust off basics like this from time to time.

Sunday 1 March 2009

Coding buddy - Interesting approach to Code Review

How often do you ask your peers to review your code? If not very often then here is an idea for you - find a coding buddy! The buddy system can be implemented in 2 simple steps. But firstly, here is the idea behind the buddy system:
Individuals can do great things, but two highly motivated peers can accomplish even more when they work together. Surely there's at least one programmer you work with who you admire or at least respect enough to adopt the buddy system with.
So step 1 is to find a person meeting the above criteria, he will be a second "half of an awesome part-time coding dynamic duo". It has been proven, that this approach really works - there are lots of well-known dynamic duos like:
Your duo can join this small hall of fame! ;)

Now it's time to move to step 2 - peer review. Let me start with clarifying what kind of peer review I'm thinking about. Peer review can have multiple variations. For sure, with you coding buddy you don't want to do very formal, systematic and rigorous reviews (inspection) you should be much more flexible and more into quick and informal meetings (peer deskcheck).


I think informal nature of peer deskcheck is the factor that makes is so effective. You can comfortably chat with your buddy and explain to him what is the business objective for given piece of code and what was your approach to the problem. It's a great way to make sure that your idea makes sense and you are on the right course.

Those are only direct benefits, indirect benefits include:
  • improved team work - reviews help people to "accustom" to helping each other
  • increased familiarity of code base - sharing knowledge is always important, reviews give developers a chance to teach others about their piece of code
  • prevent individuals from going dark
  • increased quality - "reviews motivate us to practice superior craftsmanship because we know our co-workers will closely examine our work. In this indirect way, peer reviews lead to higher quality."
And that would be it, try to implement those two relatively simple steps and check on your own if it improves your code, check if other people understand your design and learn from it.

What are the next steps? (assuming that it buddy system works)
Finally, I couldn't stop myself from posting this cartoon:


Last words ...