Tuesday, 21 February 2012

Don’t Let Your Tools Pwn You

Modern development tools such as IDEs and their associated multitude of plug-ins, such as ReSharper, can definitely improve productivity. But they can also reduce it too if welded without due care. In some cases it may not be yourself that suffers the loss in productivity but your team-mates because you were unaware of what that tool actually did.

The original impetus for this post was actually watching someone dig themselves into a right mess with ReSharper. I suspect they had got so used to accepting every pop-up that ReSharper threw at them that they didn’t notice when ReSharper had suggested the wrong change. The code didn’t compile now but it wasn’t obvious why as the wrong turn was some minutes ago and they were focused on the code they were writing now. After all, the previous code did compile so why could the mistake be back there?

Being “old school” I used the compiler output to point the finger rather than ReSharper’s advice and traced it back to the source. The earlier mistake was to mistype the inheritance of a class and then let ReSharper implement it automatically. This mistake initially swapped compile time errors for runtime errors (throw NotImplementedException is the default implementation) and unless you use a test-first approach the feedback loop between making and discovering the mistake grows rapidly.

The more recent impetus is another tale, once again involving ReSharper, that caused another person pain - me. As part of a large refactoring exercise one of my team mates had to split some core classes off into a common assembly for use by the new client application. They chose to use ReSharper for the job which makes perfect sense as refactoring is one of its main features. Sadly they didn’t know what ReSharper was also going to do at the same time...

These changes all happened in the background on the trunk and it wasn’t until I came to merge some last minute changes from our release branch that things started to get dicey. I was getting a lot of merge conflicts on the changes even though I knew that no one would have touched the code[*]. Well, I knew that the namespace at the top of the file would have changed as a consequence of the other person’s changes, but definitely no more. After looking through the Subversion log I could see that some whitespace has also changed and my gut reaction was that someone had also “fixed the formatting” at the same time as making another change. I didn’t spot this myself at first because the diff tool I originally used had “ignore whitespace changes” still set to ON after an earlier unrelated comparison! Unfortunately the piecemeal way the refactoring had been done also meant there was more noise in the SVN log[+] than a single commit.

Ultimately it took me an hour and half to convince myself that the merge was sound when it should have been trivial. I didn’t notice at the time but the unexpected whitespace changes had also destroyed the careful formatting of a table that needed to be kept in sync with a similar block in some SQL code[#]. This cost me another 30 mins or so to sort out. In retrospect I would have been better off reverting all his check-ins first, fixing the build and then merging mine in after. But that’s part of the problem with finding unexpected changes - you then go looking for the proverbial needle in the haystack that you’re convinced must exist.

My natural reaction to this second issue is that the original refactoring changes should have been diff’ed before check-in (as any change should) and then the problem would have been spotted and corrected at its source. The argument against doing that would no doubt be that they don’t have the time to diff 100 or so files and anyway the change was “automatic”. The question now is whether it would have taken that person more than 2 hours (the disruption caused to me, let alone others) to diff those files and spot that the tool was doing something unexpected? I sincerely doubt it, even doing it manually. And you could probably automate it to produce a unified diff that would clearly have shown up lots of unintended changes.

OK, so that’s only two issues, hardly enough to suggest we give up our modern tools and return to the stone age. And I’m not suggesting that. What I am suggesting is that you’re careful not to get drawn into relying on the tools so much that you cannot remember how do things “long hand”. Although I mainly use Visual Studio with ReSharper for day-to-day C# development I still like to use tools even as primitive as Notepad because they are fast, will spot issues other tools hide[$] and most importantly are even available on production hardware.

 

[*] It was the kind of legacy code you really don’t want to touch unless you absolutely have to. And if you do you want tests. But you can’t.

[+] Given the limitations of Subversion is was probably the right way to have done it as its Copy+Delete approach to moving and renaming files can get you into hot water, especially when mixed with branching.

[#] Yes, in an ideal world these two bodies of code would be generated from a single source, but life is never that easy in reality.

[$] The best way to spot a file that has a mixture of tabs and spaces is to open it in Notepad. Has anyone ever chosen 8 spaces-per-tab out of choice?

1 comment:

  1. I solved the refactoring mystery this morning. He had the PowerCommands add-in installed which defaults to "reformat on save"! I have it installed too but instinctively must have turned this off.

    ReplyDelete