Replace the Replace

I grabbed this screenshot during an online conversation about some of the more bizarre coding people have seen that came about as a result of the ongoing nasty surprises Phil Winstanley (@plip) keeps turning up in his current work.

Replace the ReplaceThis picture however shows some code that used to be in one of our systems, but was swiftly coded out when we found it. It was written by a contractor who amongst other things claimed to have worked with Microsoft writing parts of the .Net Framework.

Looking at this, if you’re a .Net Framework user you’d be forgiven for thinking that he didn’t have anything to do with the string classes, however one of his other regular complaints was how inefficient parts of the .Net Framework were, hence why he needed to spend time rewriting stuff that was already there! We could find no evidence to indicate that this function was any faster than the built in string.Replace, and since having this was a confusion and maintenance overhead it was removed.

Aside from the function being unnecessary there are a couple of other things I wouldn’t do. First up is the use of “” rather than string.Empty – Brad Abrams explains why on this short blog post. Secondly the whole routine uses systems Hungarian notation which is explicitly discouraged by the Microsoft naming conventions – ironic as it was their programmers that created systems Hungarian notation in the first place. Joel Spolsky (@spolsky) has an excellent blog post here that amongst other things discusses Hungarian notation. The third thing, which is more a matter of my personal taste is that there is a single line if statement which I would usually surround with {}, partly to make it clear, and also to help with maintenance – I’ve come across one or two difficult to spot bugs caused by missing braces, so I tend to think it’s better to use them all the time.

Also published on Medium.

Leave a Reply