views:

268

answers:

5

Which is better to use, and why, on a large project:

#if DEBUG
    public void SetPrivateValue(int value)
    { ... }
#endif

or

[System.Diagnostics.Conditional("DEBUG")]
public void SetPrivateValue(int value)
{ ... }
+8  A: 

Well, it's worth noting that they don't mean the same thing at all.

If the DEBUG symbol isn't defined, then in the first case the SetPrivateValue itself won't be called... whereas in the second case it will exist, but any callers who are compiled without the DEBUG symbol will have those calls omitted.

If the code and all its callers are in the same assembly this difference is less important - but it means that in the first case you also need to have #if DEBUG around the calling code as well.

Personally I'd recommend the second approach - but you do need to keep the difference between them clear in your head.

Jon Skeet
+1 for calling code will need to have #if statements as well. Which means there will be a proliferation of #if statements...
Lucas B
+1  A: 

With the first example, SetPrivateValue won't exist in the build if DEBUG is not defined, with the second example, calls to SetPrivateValue won't exist in the build if DEBUG is not defined.

With the first example, you'll have to wrap any calls to SetPrivateValue with #if DEBUG as well.

With the second example, the calls to SetPrivateValue will be omitted, but be aware that SetPrivateValue itself will still be compiled. This is useful if you're building a library, so an application referencing your library can still use your function (if the condition is met).

If you want to omit the calls and save the space of the callee, you could use a combination of the two techniques:

[System.Diagnostics.Conditional("DEBUG")]
public void SetPrivateValue(int value){
    #if DEBUG
    // method body here
    #endif
}
P Daddy
@P Daddy: Wrapping `#if DEBUG` around `Conditional("DEBUG")` does not remove the calls to that function, it just removes the function from IL alltogether, so you're still having calls to function that doesn't exist (compilation errors).
myermian
If one doesn't want the code to exist in release, should one wrap the method body in "#if DEBUG", possibly with an "#else" stub (with a throw or dummy return value), and use the attribute to suggest that callers not bother with the call? That would seem the best of both worlds.
supercat
@myermian, @supercat: Yes, you are both right. My mistake. I'll edit as per supercat's suggestion.
P Daddy
+18  A: 

It really depends on what you're going for:

  • #if DEBUG: The code in here won't even reach the IL on release.
  • [Conditional("DEBUG")]: This code will reach the IL, however the calls to the method will not execute unless DEBUG is on.

Personally I use both depending on the situation:

Conditional("DEBUG") Example: I use this so that I don't have to go back and edit my code later during release, but during debugging I want to be sure I didn't make any typos. This function checks that I type a property name correctly when trying to use it in my INotifyPropertyChanged stuff.

    [Conditional("DEBUG")]
    [DebuggerStepThrough]
    protected void VerifyPropertyName(String propertyName)
    {
        if (TypeDescriptor.GetProperties(this)[propertyName] == null)
            Debug.Fail(String.Format("Invalid property name. Type: {0}, Name: {1}", GetType(), propertyName));
    }

You really don't want to create a function using #if DEBUG unless you are willing to wrap every call to that function with the same #if DEBUG:

#if DEBUG
    public void DoSomething() { }
#endif

public void Foo()
{
    DoSomething(); //This will have a compilation error if you're not in DEBUG.
}


#if DEBUG
    public void DoSomething() { }
#endif

    public void Foo()
    {
#if DEBUG
        DoSomething(); //This works, but looks FUGLY
#endif
    }


[Conditional("DEBUG")]
public void DoSomething() { }

public void Foo()
{
    DoSomething(); //Code compiles and is cleaner, DoSomething always
                   //exists, however this is only called during DEBUG.
}

#if DEBUG example: I use this when trying to setup different bindings for WCF communication.

#if DEBUG
        public const String ENDPOINT = "Localhost";
#else
        public const String ENDPOINT = "BasicHttpBinding";
#endif

In the first example, the code all exists, but is just ignored unless DEBUG is on. In the second example, the const ENDPOINT is set to "Localhost" or "BasicHttpBinding" depending on if DEBUG is set or not.

myermian
+1  A: 

Let's presume your code also had an #else statement which defined a null stub function, addressing one of Jone Skeet's points. There's a second important distinction between the two.

Suppose the #if DEBUG or Conditional function exists in a DLL which is referenced by your main project executable. Using the #if, the evaluation of the conditional will be performed with regard to the library's compilation settings. Using the Conditional attribute, the evaluation of the conditional will be performed with regard to the compilation settings of the invoker.

Kennet Belenky
+1  A: 

I'm sure plenty will disagree with me, but having spent time as a build guy constantly hearing "But it works on my machine!", I take the standpoint that you should pretty much never use either. If you really need something for testing and debugging, figure out a way to make that testability seperate from the actual production code.

Abstract the scenarios with mocking in unit tests, make one off versions of things for one off scenarios you want to test, but don't put tests for debug into the code for binaries which you test and write for production release. These debug tests just hide possible bugs from devs so they aren't found until later in the process.

Jimmy Hoffa
@Jimmy great discussion starter!
Lucas B
I totally agree with you Jimmy. If you're using DI and mocking for your tests, why would you need `#if debug` or any similar construct in your code?
Richard Ev