C# conditional attribute on interface member

Question

I'm trying to get rid of the "#if TRACE" directives in my code, by using the Conditional attribute instead, but can't apply this approach easily to interfaces. I have a way round this but it's pretty ugly, and I'm looking for a better solution.

E.g. I have an interface with a conditionally compiled method.

interface IFoo
{
#if TRACE
    void DoIt();
#endif
}

I can't use the conditional attribute in an interface:

// Won't compile.
interface IFoo
{
    [Conditional("TRACE")]
    void DoIt();
}

I could have the interface method just call a conditional private method in the concrete class:

interface IFoo
{
    void TraceOnlyDoIt();
}

class Foo : IFoo
{
    public void TraceOnlyDoIt()
    {
        DoIt();
    }

    [Conditional("TRACE")]
    void DoIt()
    {
        Console.WriteLine("Did it.");
    }
}

This would leave my client code with redundant calls to the 'nop' TraceOnlyDoIt() method in a non-TRACE build. I can get round that with a conditional extension method on the interface, but it's getting a bit ugly.

interface IFoo
{
    void TraceOnlyDoIt();
}

class Foo : IFoo
{
    public void TraceOnlyDoIt()
    {
        Console.WriteLine("Did it.");
    }
}

static class FooExtensions
{
    [Conditional("TRACE")]
    public static void DoIt(this IFoo foo)
    {
        foo.TraceOnlyDoIt();
    }
}

Is there a better way to do this?

Answer 1

A trace method shouldn't be appearing on an interface as it's an implementation detail.

But if you're stuck with the interface, and can't change it, then I'd use the #if ... #endif approach that you started with.

It is a rather savage syntax though, so I sympathise with why you might want to avoid it...

Answer 2

You should leave the task to the optimizer (or JIT compiler) and use:

interface IWhatever
{
  void Trace(string strWhatever);
}

class CWhatever : IWhatever
{
    public void Trace(string strWhatever)
    {
#if TRACE
       // Your trace code goes here
#endif
    }
}

Of neither the optimizer nor the JIT compiler removes the call you should write angry emails to those developpers ;).

Answer 3

What about this:

interface IFoo
{
  // no trace here
}

class FooBase : IFoo
{
#if TRACE
    public abstract void DoIt();
#endif
}

class Foo : FooBase
{
#if TRACE
    public override void DoIt() { /* do something */ }
#endif
}

Answer 4

I would suggest you to use the null object pattern instead. I view conditional statements as a kind of code smell because they hide real abstractions. Yes you will get some extra method calls but these virtually have no impact on performance. In trace builds you can inject the TraceFoo, via a config file for example. This will also give you the capability to be able to enable it on non-trace builds as well.

interface IFoo
{
    void DoIt();
}

class NullFoo : IFoo
{
    public void DoIt()
    {
      // do nothing
    }
}

class TraceFoo : IFoo
{
    public void DoIt()
    {
        Console.WriteLine("Did it.");
    }
}

Answer 5

I like the extension method approach. It can be made a bit nicer/robust, at least for callers:

    public interface IFoo
    {
        /// <summary>
        /// Don't call this directly, use DoIt from IFooExt
        /// </summary>
        [Obsolete]
        void DoItInternal();
    }

    public static class IFooExt
    {
        [Conditional("TRACE")]
        public static void DoIt<T>(this T t) where T : IFoo
        {
#pragma warning disable 612
            t.DoItInternal();
#pragma warning restore 612
        }
    }

    public class SomeFoo : IFoo
    {
        void IFoo.DoItInternal() { }

        public void Blah()
        {
            this.DoIt();
            this.DoItInternal(); // Error
        }
    }

Generic type constraints are used to avoid the virtual call and potential boxing of value types: the optimizer should deal with this well. At least in Visual Studio it generates warnings if you call the internal version via because of Obsolete. An explicit interface implementation is used to prevent accidentally calling the internal methods on the concrete types: marking them with [Obsolete] also works.

While this may not be the best idea for Trace stuff, there are some cases where this pattern is useful (I found my way here from an unrelated use case).