The established wisdom – and there’s undoubtedly a lot of truth in it – is that catching a bug late in the development cycle can cost as much as 1000 times as much to fix as the same bug would have cost, if you’d caught it earlier. Really bad bugs can even (depending on what you’re making) make you miss your Christmas launch date, fail to reach launch altogether, force a product recall after launch, or even open you up to legal action. Some bugs have cost their authors literally millions of pounds, and the hardware bug that causes the Xbox 360 “Red Ring of Death” cost Microsoft billions. Of course, “cost” could equally mean time instead of money – but in either case, I’d rather spend ten minutes fixing a bug just after I wrote it, than wait till just before releasing my game and realise I have to spend hours, days or weeks finding and fixing the same bug.
The first weapon in your armoury to find and fix bugs early is to turn the compiler warnings up to the most practical maximum, which in the case of Visual Studio means Warning Level 4, or /W4. (If you’re stuck using Visual C++ you might be surprised to learn that some amazingly useful, in fact, critical warnings are actually disabled by default at level 4). You should always ensure your code compiles without warnings at /W4. You may think some warnings are unnecessary; fix them anyway. It won’t hurt, and might even help without you realising.
Once you’ve got your warnings turned up and fixed, the next step is to use static analysis tools like FxCop. Static analysis tools find bugs in your code without you even needing to run it. In fact, the next version of FxCop isn’t even a separate tool, it’s a compiler option (/analyze). These tools parse your code (or in FxCop’s case, examine the MSIL assemblies) and compare what you’ve written against a set of rules that help to identify dangerous patterns in your code.
With FxCop installed, you simply need to open it up, start a new project, and point it at where to find the assemblies (.dll or .exe) for your code. It works equally well with either XNA game code or standard .NET non-game code. Hit “Analyze”, and a few seconds later it will come up with a list of problems that it has detected, along with an explanation about why they are problems and suggestions on how to fix them.
You will probably be shocked and surprised by how many problems it finds in your code. The good news is, it’s probably not as bad as it looks; the bad news is, that’s because FxCop is slightly on the noisy side, though you have complete custom control over which rules you enable (and you can even define your own). For example, FxCop regularly complains at me about the following:
IdentifiersShouldBeSpelledCorrectly: Correct the spelling of ‘Kensei’ in namespace name ‘Kensei’.
Well, first of all, Kensei is the correct spelling, it’s the codename of my engine. Secondly, I’m not completely sure that I care if the spelling is correct – nobody is going to play my game and think, “it’s a good game, but he misspelled one of his namespace names”. Thirdly, I’ve followed the instructions to create my own user dictionary for FxCop to add “Kensei” to it, and couldn’t get it to work; and I’m certainly not going to try and think up a whole new codename for my project just to keep FxCop quiet. So, I usually disable this rule, or at least exclude the specific occurrence of the warning.
There are some warnings that are a little bit trickier and need more thought. Like this one:
MarkAssembliesWithClsCompliant: Mark ‘Kensei.dll’ with CLSCompliant(true) because it exposes externally visible types.
Really…? ‘CLS compliant’ means, basically, that the assembly can be used by other languages in the Common Language Specification that aren’t C#, like Managed C++ or Visual Basic. I guess, on one level, that seems like a pretty noble thing to aim for, though I wouldn’t expect to ever use those other languages given that I love C# so much. But it’s possible I might make parts of my code available to other people, and they might want to use those other languages. So should I follow this advice or not?
In this case the answer comes when I do what it says to do. Marking the assembly as CLSCompliant results in a number of compiler warnings, because I’ve been using types that are not CLS compliant. That’s fair enough, except that these types include things like GameTime, which is a core XNA type. The XNA team chose not to make their assemblies CLS compliant; so there is absolutely no point in me trying to do so.
Anyway, by now you’re probably wondering why I’m talking about FxCop when so far I’ve just described some things it does less than amazingly well. As it happens, I think it’s a wonderful little tool, especially since it’s completely free. I run it on all my code – the Kensei engine, and my games, including Pandemonium – regularly; at minimum, after each “milestone” I’ve set myself. I’ll then fix the problems it highlights, either by changing the code as recommended or by marking a warning as Excluded, to say that the infraction was deliberate. So… why? Here’s why (a real-life example, though the names have been changed to protect the innocent):
public class BaseClass
protected virtual void InitValues()
// Rest of class implementation
public class DerivedClass : BaseClass
protected override void InitValues()
// Rest of class implementation
Spotted the error yet? You probably should have done – I’ve certainly stripped it down far enough! When I wrote this, the BaseClass was in BaseClass.cs in a project in my Kensei solution, and the DerivedClass was in DerivedClass.cs in my Pandemonium solution, and both files were pushing 1000 lines of code. So the problem was a lot less obvious… but no less real for all that. FxCop spotted it straight away:
DoNotCallOverridableMethodsInConstructors: ‘BaseClass.BaseClass()’ contains a call chain that results in a call to a virtual method defined by the class. … Virtual methods defined on the class should not be called from constructors. If a derived class has overridden the method, the derived class version will be called (before the derived class constructor is called).
In other words, when I did something like this:
DerivedClass derived = new DerivedClass();
… it would (conceptually; this isn’t completely accurate) first allocate memory on the heap, then call the BaseClass constructor, the BaseClass constructor would call the DerivedClass virtual InitValues method, which would call into the BaseClass InitValues method, and only then would it call the DerivedClass constructor before giving me back the reference to the object. That is to say – it would be calling a member function intended for a DerivedClass object, on an object that wasn’t a DerivedClass at all, only a BaseClass.
Most of you will be well aware that this is a Bad Thing. I certainly was already. Such behaviour is not polymorphism (a good and noble ideal), it’s just broken and wrong. It could rely on members being in a certain state that weren’t in that state yet. It could call out to other functions in other objects, and end up interacting with them despite being only partially constructed. It could, frankly, do just about anything, and almost none of the things it could do would be good. (For more on what is allowed in C#, see the C# Standard). However, despite the fact I’ve been programming for [mumblemumblemumble] years in total, and for six of those years I’ve been paid to do it, I still made this mistake. Well, I am only human after all – and we humans make mistakes.
It’s a mistake that FxCop spotted and I’ve since fixed it. It spotted it almost as soon as I’d written it, and it took five minutes to fix. Late in the dev cycle, I might only have spotted it after hours trying to work out why my DerivedClass objects were behaving strangely, and so much other code might have become dependant on that behaviour that it might have taken hours longer to fix. I could have lost days or weeks to one silly little bug – and believe me, I’ve seen exactly that happen before now.
There are many other genuinely code-breaking bugs that the compiler can’t spot but FxCop will pick up immediately. (Have you implemented the Dispose pattern correctly throughout your program? Are you really sure about that? How about private members and functions that are never used?) If you’re willing to spend money, there are other static analysers out there that check correctness and performance at an even lower level, therefore finding even more subtle (or critical!) bugs, and all without actually running a single cycle of your code. (Indeed, if you’re using C++ for some reason and not using Lint, BoundsChecker, PREFast or something similar, I bet real money your code is broken). But for C# and/or XNA, FxCop is free, easy to use, easy to customise, and if you’ve not used it yet I strongly recommend that you start right now.
“If debugging is the process of removing bugs, then programming must be the process of putting them in.” – Edsger Dijkstra