Specifying properties using Lambda expression trees instead of Reflection

For my TranslationTester project I need users to be able to specify properties on classes so that they can specify that one property maps to another, and to indicate which property they are mapping (or even excluding).

Up until now I’ve only been able to achieve this using strings that match the property names. This is bad because a) it’s difficult to use, and b) it doesn’t support refactoring very well – (although the tests give helpful exceptions when a property has been renamed it would still need to be manually corrected)

Whilst attending DDD7 I saw a demo on LINQ by Jon Skeet and saw a type of Lambda expression I’d not really seen before that was being used to specify a Key on  class. This got me to thinking that this is basically what I want to do. Looking into it I found a few other resources explaining how to do this.

Here’s what I came up with:

target.AddMapping(f=>f.Property1,t=>t.Property1);

which is achieved by:

public SimpleMapping<TFrom, TTo> AddMapping<TProp>(
  Expression<Func<TFrom, TProp>> fromProp,
  Expression<Func<TTo, TProp>> toProp)
{
  var fromName = fromProp.ToPropertyName();
  var toName = toProp.ToPropertyName();
  return AddMapping(fromName, toName);
}  

...

public static class ExpressionExtensions
{
    public static string ToPropertyName<TFrom, TProp>(this Expression<Func<TFrom, TProp>> propertyExpression)
    {
      var lambda = ToLambda(propertyExpression);
      var prop = lambda.Body as MemberExpression;
      if (prop != null)
      {
        var info = prop.Member as PropertyInfo;
        if (info != null)
        {
          return info.Name;
        }
      }
      throw new ArgumentException();
    }    

    private static LambdaExpression ToLambda(Expression expression)
    {
      var lambda = expression as LambdaExpression;
      if (lambda == null)
      {
        throw new ArgumentException();
      }
      var convertLambda = lambda.Body as UnaryExpression;
      if (convertLambda != null
          && convertLambda.NodeType == ExpressionType.Convert)
      {
        lambda = Expression.Lambda(convertLambda.Operand, lambda.Parameters.ToArray());
      }
      return lambda;
    }
}

Note: Some of this is borrowed/inspired very heavily from Moq, particularly the conversion of lambda convert expressions.

One of the nifty things about it is that the syntax of the two Expressions enforces a lot of the requirements on the property types. By using TProp the properties that are referenced in the expressions must be of the same (or common base) type.

I’ve been using the new way of referring to properties and I must say I’m really happy with it!

Code review, before or after check-in?

Recently I’ve been thinking about our code review process, and how it could be improved. We’ve recently moved from a simple VSS based source control to a more feature rich TFS based approach.

We now have the majority of feature development occurring in several development branches, at the end of each development iteration we get all the changes reviewed and then merge the changes into our Main (or trunk); a build from Main will then be passed to test.

Problems we’re finding with this approach are:

Development Code Freeze

Whilst code is being reviewed (and review actions being actioned) we pretty much freeze feature development in the branch, the code review can often take a long time, so people often start doing further development off-line, ready to check-in once the freeze is lifted.

How can we make sure that what is eventually merged into Main is what was reviewed? 

If the review is done against the changes between Dev and Main then you miss reviewing any merge conflict resolutions (unless you do a 2nd review). If you review the ‘pending merge’ (where someone does all the merge process apart from the final check-in to Main), you risk the merge no longer being valid (someone else checks into Main between pending merge and review complete).

Seeing very little benefit from the review

Our process of reviewing all code going into Main is giving us a false sense of confidence, as we think Main has top-quality code in it; in fact this is not the case, often it seems the only review comments that are actioned are fairly trivial changes that wont take very long. Any fairly major changes are often deferred to a future ‘Change Request’, with the merge going ahead anyway, as the feature is required.

We do want to prevent breaking changes going into production or even to test, but this should probably be caught by testing (automated) and Continuous Integration.

Code review is inconsistent

Code reviews are processed in a fairly inconsistent out-of-band way, with very little formal process around it. Review comments can sometimes be lost, and tracking whether the review was fully actioned etc is not 100% possible.

Suggestion: Do the code review after the check-in to main

So the suggestion I’ve been making is that instead of holding up the check-in to main with the code review we do the code review afterwards. The process would be something like this:

• Feature development is done in a development branch
• Towards the end of the iteration the code is prepared for check-in to main
• The code is merged into main
• The check-in to main causes a code review task to be created for the changeset
• Code review actions are done directly against main.
• Once the code review has all been resolved (actioned, further tasks raised etc). It is marked as closed.
• Before main is pushed to production all feature development code reviews must have been completed.

So if we adopted this process would it resolve our issues?

• Development Freeze – by decoupling review from development the two activities could run in parallel, as soon as the merge is complete development could resume. If merging up to a label you wouldn’t even have to freeze whilst the merge is done.
• Review exactly what was merged – by definition we would be reviewing what was merged. We could even say that review actions should be reviewed so no changes to main go unreviewed.
• Review benefit – by decoupling review from development and making the review process more traceable I think we will be able to add review actions to the product back-log and then even the big changes will be done. Review actions should be treated in a very similar way as feature development with priority dictating what is actually done.
• Review inconsistency – having a proper review process that is tracked with tasks will means a more consistent approach and a reliable process. Any unresolved review comments will be searchable/reportable just like any other tasks.

So what would the cons be?

• I think we would risk checking in bad code to main, and although we could use our source control system to roll these back etc, we might not catch them in time – I don’t think, however, that our current process really stops this happening.
• Reviewing could be neglected in favour of feature development, but this just means that project management needs to see the importance of the review process and prioritise the tasks appropriately.

Accepted Best Practice

Before writing this post I tried to search for the best practice in this area and couldn’t really find any. I guess one answer is that instead of code reviewing paired programming could be more effective. Does any one have any suggestions as to what others are doing?

How does Silverlight fit into a ‘proper’ UI architecture such as MVC?

With Microsoft releasing a whole caboodle of new tools as part of VS2008 and .net 3.5 I thought it would be fun sensible to spend some time trying to figure them out and having a play. I also thought that I would spend some time playing about with some UI tools/best practices that I had heard about but never used (such as MVC and Monorail).

My grand idea is to create a system to act as a Video File Organiser, I have some initial thoughts about what I want it to do, but the main reason to do it is to play with the tools.

I want to create a Silverlight UI, working in a proper MVC architecture that uses an Astoria web data service to query the library. At some point I’ll also think about how I would like to populate the library with video files on the server.

Problem 1 - Silverlight with MVC

Granted, Silverlight looks fantastic, but I haven’t been able to find much about how to combine it with proper architectural best practice. What I would like to do is figure out MVC and Silverlight at the same time, and see if it makes sense to use Silverlight as the View.  This is all just guesswork at the moment, and my first task is to try and familiarise myself with the basics of these technologies before I start trying to combine them in weird and wonderful ways.

I assume I could ‘do’ MVC from scratch, but I wonder if it would be possible to use a framework such as Monorail? Monorail doesn’t seem to have any mentions of Silverlight, so might have to figure this one out by myself. It seems that Silverlight would make sense in one of two places -

1. Create a new Monorail view engine for silverlight - this would somehow create the silverlight markup…
2. Use an existing view engine – not sure what view engines really are, but could NVelocity create Silverlight already if you specify the appropriate template?

Either way, how do you connect the controller to the silverlight generated…. hmmm

Dear reader, I will report back once I have investigated.

Mocking without a mocking framework

I have not yet had the opportunity to use a proper mocking framework. My project has a future investigation into them, but it’s a low priority. However we are using Test Driven Development, so we have started using mocks (with dependency injection), so far the mocks have been statically defined and we are ‘learning as we go’ in the sense that the quality of the mock is changing as is our technique for creating them. We started off with a mock of a database with hard coded data in it, if you like we were replicating a simple set of data that the mock would interrogate. Over time this became very messy and was difficult to change for a specific test as you could break an existing test. The next approach was to remove all hard coded data and provide a mock that could be dynamically loaded with data by the test. This seems better (and more like how mocking frameworks work I believe). However recently I realised the method for loading data was flawed as if a class called the same method twice, but required different output on each call then the mock would not be able to provide this. The following shows my current thinking for how to define a bespoke mock with dynamically loaded data. Using a queue to load the data for each call.

1. Create your mock implementing the mocked object(s) interface(s)
2. Create a field/property of type queue<type> for each type of data that needs to be returned
3. Create a constructor for the mock that news all the queues
4. For each method on the interface(s) take the appropriate queue and Dequeue() an item from it

public class MockDB:ICustomerDataAccess
{
    #region Dynamically loaded results

    private Queue<List<Customer>> loadedCustomerListQueue;

    public Queue<List<Customer>> LoadedCustomerListQueue
    {
        get { return loadedCustomerListQueue; }
        set { loadedCustomerListQueue = value;}
    }

    private Queue<Customer> loadedCustomerQueue;

    public Queue<Customer> LoadedCustomerQueue
    {
        get { return loadedCustomerQueue; }
        set { loadedCustomerQueue = value; }
    }

    #endregion

    public MockDB2()
    {
       LoadedCustomerListQueue = new Queue<List<Customer>>();
       LoadedCustomerQueue = new Queue<Customer>();
    }

    #region ICustomerDataAccess Members

    public List<Customer> GetCustomerByForeignKey(long ID)
    {
        return LoadedCustomerListQueue.Dequeue();
    }

    public Customer GetCustomerById(long id)
    {
        return LoadedCustomerQueue.Dequeue();
    }

    #endregion
}

To use the mock in a test:

1. Create an instance of the mock
2. Enqueue items into the appropriate queue in the order that they should be returned in the tested class.

[TestMethod()]
[ExpectedException(typeof(CustomerMaintenanceException))]
public void UpdateCustomer_DoesntExist_Fail()
{
    MockDB Mock= new MockDB();
    UpdateCustomer target = new UpdateCustomer(Mock);
    Customer NonExistant=new Customer();
    Mock.LoadedCustomerQueue.Enqueue(null);
    target.Execute(NonExistant);
}

Some points to note:

• You can enqueue null values onto a queue
• You can have a queue of lists
• This only works if the tested class has a predictable logic flow, but for a proper unit test this should always(?) be the case
• This is not meant to be better or even anywhere near as good as a mocking framework, but where you can’t use a framework this approach seems sensible

Update: As my knowledge of mocking has improved I have realised that this is not really mocking, rather a sort of dynamic stubbing (see this post)

How many tests should I write for a unit

When coding using Test Driven Development (TDD). It is sometimes tricky to figure out how many tests you should write. (Forgetting about the whole tests vs. asserts debate). I believe the guideline should be that the minimum amount of tests you should write is the number required to get 100% of the unit code covered. This means that every logic branch of the code is tested. From a requirements point of view this will equate to one test per possible type of successful outcome and one test per exception explicitly thrown in the code (failure outcomes). I am sure that this is not true in all cases, and it is really just a minimum.

Is consistency of method within a solution essential?

As a developer I find that I am constantly finding out about new development methods, constructs and techniques. I don’t know if this is because I am relatively ‘fresh out of uni’ or that I am working in a ‘new’ technology area (e.g. .net3.0) << how many dots?!>> Or whether this is just how development is.

When I find a new method I tend to implement my current task using the new method leaving existing parts of a solution using the ‘old’ method. This can result in a somewhat inconsistent solution, with different areas of the solution using different methods.

Is this a ‘bad’ thing? probably, ideally you would have implemented the whole system using the ‘better’ method from day 1. But is it better to re-do areas of a solution that are working fine as they are? this will take extra effort, and could introduce bugs unneccessarily. So if you don’t re-work all areas of the solution should you resist using new methods within a solution thereby keeping consistency, but not taking advantage of better techniques. Also introducing new methods can be irritating for other members of a development team as they also must understand the new method to be able to work effectively with the solution.

Is it ok for a single solution to use many different building blocks? lots of different methods all working together? It is probably not ideal and I guess this is the purpose of having lead developers and architects who are aware of the general development community and make educated decisions as to which methods will be used and which will be vetoed. But does this mean that non-lead developers should just be code-monkeys? should they not be interested in promoting techniques and methods that aren’t in the existing architecture?

So what do I think? Hmm well I’m not sure, to be honest I liberally go through existing solutions and use new techniques freely with little consideration for the rest of the solution (of course I check everything still works and I haven’t broken existing tests/functionality). However I think that developers should be encouraged to investigate new techniques, but they should not be introduced to production code without discussing the technique with the lead developer or the rest of the team. I do not think that mixing and matching within a solution is definately bad, and in some ways it is a demonstration of decoupling in a solution. The total number of methods in a solution should be managed so as to maintain the maintainability of the solution, so each new method should be judged and a decision made to do one of the following options:

• Not use the method at all
• Not use the method in existing solutions but use in new solutions
• Use the method in all solutions, without reworking existing areas
• Use the method in all solutions reworking solutions completely to the new method

Whatever choice is made effective communication to the whole development team is essential.