Know your code

18 July 2017

Photo: Unsplash

“We learn from failure, not from success!” - Bram Stoker.

Or

From a long career, lots of lessons have been learnt which are worth sharing.

One lesson that I try to share with developers is that you have to understand how your code works. That is you need to know how it will work within the execution environment. If your code runs in the terminal it will work in a different manner than if it runs on a web server. Distributed applications need extra consideration and knowledge to ensure expected execution. Different framework versions even handle similar things in a different way. This is something that is lacking in a world of Google and StackOverflow. People paste code without fully understanding what they are adding to their code base.

Recently, a developer was trying to pass data to a class via the AppSettings mechanism. This is a NameValueCollection used to store Application Settings for the application. The code base had an abstraction for the Application Settings. The developer wanted to add one key-value pair to the AppSettings collection. This was to be able to use the data in a service class which he thought didn’t have access to the Session data. Now it is clear that this is not a clean solution. The thing that struck me was more that the developer didn’t realise that the code could never work. This code was to be in run in a load balanced, web application environment with a shared session state provider.

The values that the developer was adding to the AppSettings came from the Session. Each time the Session value changed it would write the value to the AppSettings collection. This is where it was going wrong. While writing the code it functioned fine. They would use single test accounts and the value would update as expected and the service class worked as required.

Then the developer assigned me a Pull Request to me for review. The first thing that I decided, having read the code, was to test the code with multiple Sessions. If you have code that is mapping Session data to an application state collection then it seems logical to test the code with many users. Three browsers and three test users later I had proved what I already knew and proved that the code would not work for many users. The first user’s data gets added to the collection and all is well. Then the second user’s data overwrites the first user’s data in the Application Settings during use. The service class attempts to access the data for the first user and instead gets the data added for the second user. Oh dear.

In talking to the developer they admitted that they had tested with one browser and one user. They also admitted to not fully understanding how the AppSettings code worked within the lifetime of a request. Ironically, the solution was to use the ASP.Net Session library as there was already a reference in the Service library.

It may seem a waste of time to let the developer get to a Pull Request without intervening. I disagree and think that within reason, allowing a developer to make a mistake and learn from it has enormous benefit. In this case, they learnt a lot about the configuration mechanism and load balanced environments. They also learnt to fully understand the implications of their code before pushing it for review.