If You’re Using “#if DEBUG”, You’re Doing it Wrong

I was going through some legacy code the other day, refactoring it all over the place and I ran into many blocks of code wrapped in “#if DEBUG”.  Of course, after a bit of refactoring in a RELEASE configuration these blocks of code were quickly out of date (and by out of date, I mean no longer compiling).  A huge PITA.

For example, take the following code:

	public class MyCommand
	{
		public DateTime DateAndTimeOfTransaction;
	}
 
	public class Test
	{
		//...
		public void ProcessCommand(MyCommand myCommand)
		{
#if DEBUG
			if (myCommand.DateAndTimeOfTransaction > DateTime.Now)
				throw new InvalidOperationException("DateTime expected to be in the past");
#endif
			// do more stuff with myCommand...
		}
		//...
	}


If, while my active configuration is RELEASE, I rename-refactor DateAndTimeOfTransaction to TransactionDateTime then the block of code in the #if DEBUG is now invalid—DateAndTimeOfTransaction will not be renamed within this block.  I will now get compile errors if I switch to DEBUG configuration (or worse still, if I check in an my continuous integration environment does a debug build).



I would have run into the same problem had I been in a DEBUG configuration with all the “#if RELEASE” blocks.  Yes, I could create a configuration and define DEBUG and RELEASE and do my work in there.  Yeah, no, not going down that twisted path… i.e. try doing it without manually adding a conditional compilation symbol.



I got to thinking about a better way.  It dawned on me that #if blocks are effectively comments in other configurations (assuming DEBUG or RELEASE; but true with any symbol) and that comments are apologies, and it quickly became clear to me how to fix this.



Enter Extract Method refactoring and Conditional Methods.  If you’ve been living under a rock or have simply forgotten, there exists a type in the BCL: System.Diagnostics.ConditionalAttribute.  You put this attribute on methods that[whose calls] may or may not be included in the resulting binary (IL).  But, they are still compiled (and thus syntax checked).  So, if I followed the basic tenet for code comment smells and performed an Extract Method refactoring and applied the ConditionalAttribute to the resulting method, I’d end up with something like this:



		//...
		public void ProcessCommand(MyCommand myCommand)
		{
			CheckMyCommandPreconditions(myCommand);
			// do more stuff with myCommand...
		}
 
		[Conditional("DEBUG")]
		private static void CheckMyCommandPreconditions(MyCommand myCommand)
		{
			if (myCommand.DateAndTimeOfTransaction > DateTime.Now)
				throw new InvalidOperationException("DateTime expected to be in the past");
		}


 



Now, if I perform a rename refactoring on MyCommand.DateAndTimeOfTransaction, all usages of DateAndTimeOfTransaction get renamed and I no longer introduce compile errors.



Code Contracts



If you look closely at the name I chose for the extracted method you’ll notice that what this DEBUG code is actually doing is asserting method preconditions.  This type of thing is actually supported in Code Contracts.  Code Contracts implement a concept called Design by Contract (DbC).  One benefit to DbC is that these checks are effectively proved at compile time so there’s no reason to perform this check at runtime or even write unit tests to check them because code to violate them becomes a “compile error” with Code Contracts.  But, for my example I didn’t use DbC despite potentially being a better way of implementing this particular code.



Anyone know what we call a refactoring that introduces unwanted side-effects like compile errors?



[UPDATE: small correction to the description of conditional methods and what does/doesn't get generated to IL based on comment from Motti Shaked]

15 thoughts on “If You’re Using “#if DEBUG”, You’re Doing it Wrong”

  1. Nice post dude. I’ve been using Conditional() for a while now and started for the same reason you did, because of compile errors after refactoring depending on which configuration I was working with.

    Using Conditional() cleans up the code and you get a nice separation of your “debug” stuff which is a nice side effect as well.

    Cheers.

  2. Hi Peter,

    Nice post! Thanks for reminding us about the Conditional attribute, I feel that not enough programmers keep it in mind when developing. There is a small inaccuracy I wanted to bring to your attention.

    While Conditional can be very useful, it isn’t equivalent to preprocessor directives. It is most useful in libraries.

    You wrote “You put this attribute on methods that may or may not be included in the resulting binary (IL).”

    The documentation stipulates: “Applying ConditionalAttribute to a method indicates to compilers that a call to the method should not be compiled…”.

    A method with the Conditional attribute is ALWAYS generated into IL. You can check that using ILDASM. The CALL to the method potentially isn’t.

    The idea behind the Conditional attribute is that the end result (whether the method is invoked) depends on the compilation settings of the caller’s code, rather than the code that includes the method. This can be very useful for libraries. For example, System.Diagnostics.Debug.WriteLine is compiled with the conditional attribute. The code for this method is present in the System.dll assembly. If you write code that calls this method, the compiler will emit the call only if you compile in DEBUG.

    Having said that, you can still use Conditional instead of #if as long as you don’t mind that unused code is included in your output assembly.

    Again thanks for the post, your blog is great!

  3. Hi Motti. Thanks for the comment. You are correct, I should be more clear and say the “call” to the method is not included in the IL not the method itself. Thanks

  4. Yet another article where the author claims that doing something is wrong automatically when it can be totally okay. Going for shock titles instead of good advice is the new trend I suppose. Codinghorror.com 2.0…

  5. My own rules concerning contracts:
    –>Use it only for publicly visible methods to be used by clients other than yourself
    Unfortunately tooling that rewrites assemblies to include MS code contract is slow, it takes more time to rewrite the asm and include contracts in IL, than the C# compiler takes to compile the asm! More than doubling my compilation time is no an option.
    So I use contract API only on asm dedicated to be used by clients other than myself.

    For contracts inside non publicly exposed methods, I use the good old Debug.Assert(), that saved me thousands of time for decades.
    And when it comes to using Debug.Assert(), I found it legal to sometime have smthing like:

    #if DEBUG
    bool b = {check a complex state here}
    Debug.Assert(b, “Complex state violated”);
    #endif

  6. I’d be tempted to think that most of us work in debug mode until compiling a release build. Refactoring in release is just asking for trouble and I’ve never used #if !DEBUG to -add- code in release mode, so if you work in DEBUG the entire time you never hit the problems mentioned in the article.

  7. There are some cases where this does not work, e.g.:

    #if DEBUG
    var foo = …
    #endif

    // production code

    #if DEBUG
    Debug.Assert(foo == bar);
    #endif

    Conditional attribute is useless because foo is a local variable that will be used in another conditional block.

  8. Don’t agree with the title, as #if DEBUG is stupid-powerful if you use it in a way that makes sense. However, using it for simple precondition checks is really quite useless, and Conditional() is a great way to fix this easily, although I would beware of hidden dependencies and the introduction of Heisenbugs due to this. I do a lot of what I do in c++ these days though, and although I hate macros, they’re stupid-useful for something things and, when you are clever with them, they can save you quite a bit of time.

  9. Another valid reason to use #if DEBUG instead of Conditional() is if the debug statements contain code that, leaked into the wild, could compromise your security.

    The #if DEBUG avoids this by not allowing the code to compile into the module in the first place. Thus it can only exist in debug builds, which should never be in the wild (although I’ve seen a heck of lot of it out there, even in expensive commercial products). This was true in C++ and is especially true in C# where things like Reflector will quickly bark out fairly readable source code from a program with no effort at all.

    The proper answer is, as is typically the case, “It depends.” Anybody who claims to know the One True Way typically hasn’t thought the issue all the way through. Hard and fast rules typically can be hard and fast *only* because they’re incomplete and ill-considered.

  10. Resharper can help rename-refactor all class at one time – ignore the #if definition.
    Simply check “Search in comments and string literals” in the Resharper-Rename window.

  11. “Of course! Many magic tricks occur well before the trick is seen to be happening. The audience was looking at you and him. And not looking at the boxes. That was when the illusion occurred.”

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>