Twit Tips #1: Parse the Time Away

Posted by ryansouthgate on 16 Nov 2018

Twit Tips

I’ve seen some software funnies in my time, in this on-going mini-series I’m going to be sharing them with you. These posts will all contain genuine code I’ve seen personally, in production systems, (or even code I’ve written myself) - I’ll leave the guessing who wrote it up to you!

I’ll hopefully highlight some “interesting” code I’ve found in systems, but also aim to show you that developers of all ages and levels of experience make mistakes/bad judgements from time to time. We shouldn’t be embarrassed by these. Instead, have a chuckle and then get them fixed.

If you come across any you think are particularly hilarious, please let me know on Twitter and I’ll feature them here.

#1 - Just “Parse()-ing” Time

public void SendXml(XElement xEl)
{
    var client = new WebClient();
    // Removed bunch of other code which adds loads of auth/custom headers to the webclient object
    
    // Buried inside a 35 line method was this little gem...
    var serviceResponse = client.UploadString("https://url.org/params", XElement.Parse(xEl.ToString()));
}

The method above, was about a 35 lines long and had a bunch of if statements in - all fair and square. However, buried in the code was the XElement.Parse(XElement.ToString()). On first glance, this isn’t too bad, probably a simple copy and paste error from a developer in a rush. The size of data this XElement held varied, but it was usally well over 40 lines long. When this specific method is called a lot of times - things certainly aren’t going to be too quick.

In the system, this was called a lot of times, for example, when hitting the home page there would be around 13 external http calls being made by the server, each of those calls would pass through this method. And with multiple users on the site at once, this execution number rises rapidly.

I came across this method, during some work to extend the class this method belonged in. I noticed this line and simply removed the Parse. When our next release went out, we ran the suite of UnitTests, then deployed it to run our performance tests - the external calls were executed quicker (over 40% IIRC) and the system was able to handle more traffic at once! Win Win!



comments powered by Disqus