Keep on tweaking – solution

in my previous post I published the following code

   1: public void SetCustomerPropertiesOnView(IEnumerable<Customer> customers)

   2: {

   3:     int count = 1;

   4:     string previousSurname = string.Empty;

   5:     foreach (var customer in customers)

   6:     {

   7:         this.View.AllCustomers.Add(new CustomerDisplay(customer));

   8:  

   9:         if (count == 1)

  10:             previousSurname = customer.Surname;

  11:  

  12:         if (customer.Surname != previousSurname)

  13:         {

  14:             this.View.SurnameFilter.Add(new NorthWindListItem(customer.Surname, customer.ID.ToString()));

  15:             previousSurname = customer.Surname;

  16:         }

  17:         count++;

  18:     }

  19: }

The context around this code is a view that shows all customers and allows the user to filter on surnames. Therefore I go and get the customers from the database I transform them for display and extract the unique surnames for the filtering functionality. I want to do all of this in one iteration of the customer list.

The above code achieves this aim, it may well be a naive implementation and there are other ways of doing this in a more efficient manner. That said the point of this post is to focus on the maintainability of this implementation, not it’s efficiency or approach. I would say that most developers would probably leave this implementation as is once they have achieved the goal. They would probably not be inclined to refactor this working solution into a more elegant solution that is maintainable over time.

My end solution to this problem looks like this

   1: public void SetCustomerPropertiesOnView(IEnumerable<Customer> customers)

   2: {

   3:     var customersAndSurnames = customers.TransformForDisplay().AndGetDistinctSurnames();

   4:  

   5:     this.View.AllCustomers = this.GetCustomersFrom(customersAndSurnames);

   6:     this.View.Surnames = this.GetUniqueSurnamesFrom(customersAndSurnames);

   7: }

The implementation of the two extension methods is not really that important in the end we still have the horrible implementation, but at the top level when the next guy comes along to add or maintain this code they will have some context around what we are trying to do. SetCustomerPropertiesOnView now has a more declarative style, you can see straight away that we are transforming customers for display and getting distinct surnames in order to give them to the view. The two private methods are just wrappers around extraction of the values in a C# implementation of a tuple, which is used to return two different types of values from one method without using out or ref parameters.

In conclusion once you have found a solution that works don’t just stop there, use the tests too aid your refactoring of a first attempt into a more maintainable elegant intention revealing block of code.

Keep On Tweaking

What does this piece of code do?

   1: public void SetCustomerPropertiesOnView(IEnumerable<Customer> customers)

   2: {

   3:     int count = 1;

   4:     var previousSurname = string.Empty;

   5:     foreach (var customer in customers)

   6:     {

   7:         this.View.AllCustomers.Add(new CustomerDisplay(customer));

   8:  

   9:         if (count == 1)

  10:             previousSurname = customer.Surname;

  11:  

  12:         if (customer.Surname != previousSurname)

  13:         {

  14:             this.View.SurnameFilter.Add(new NorthWindListItem(customer.Surname, customer.ID.ToString()));

  15:             previousSurname = customer.Surname;

  16:         }

  17:         count++;

  18:     }

  19: }

 

I will explain the reasons behind this code, and come up with a more refined solution in a later post. More often than not we come across implementations like this where the maintainer has to work really hard to get to the intent of the code.  In short this is an example of working code that is not easily maintainable over time, we can and should do better. With the safety net of unit tests I can now play around with this code to make it better.

This all goes back to Mike Waggs post on why he loves TDD/BDD particularly when he talks about tweaking code

“The best developers are those that continuously refine their code. They take something that is just a solution to the problem at hand and transform it into something which is clean and elegant. They craft a piece of code that is easily understandable by other developers and which can be easily maintained as the requirements of the system changes.”

I couldn’t agree more with this sentiment. The code above is a practical example of why we should strive to keep on tweaking. As I said before the code works, it does a job, should I walk away happy with this implementation. Absolutely not, we can make it better, so that the next person that has to delve into this code will get more understanding from reading it, not more questions.

Trying to understand this implementation will take time, to the business time is money and developer time is even more money. So for the good of the business don’t lock in your ignorance by leaving a first attempt at a solution to a problem, tweak it, until it clearly expresses your intent.

My solution will follow…what’s yours?