8

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?

Ergwun
  • 12,579
  • 7
  • 56
  • 83
  • Would using [partial methods](http://msdn.microsoft.com/en-us/library/wa80x488.aspx) be useful for you here? – Jeff Mercado Mar 09 '11 at 05:40
  • 5
    I feel like if you're trying to do this to interfaces you might have a leaky abstraction somewhere. Trace information might be more of an implementation detail than an contract detail. – Roman Mar 09 '11 at 05:47
  • @ROMANARMY: Yeah, it's a pretty horrible interface. I'm just trying to sanitise it one step at a time. – Ergwun Mar 09 '11 at 05:59
  • 2
    +1 @ROMANARMY. A trace method shouldn't be appearing on an interface as it's an implementation detail. But if you're stuck with it, I'd use the #if ... #endif approach that you started with. – sheikhjabootie Mar 09 '11 at 07:33
  • @Xcaliburp. Yeah, I think I'll just stick with the #if's until I've removed the trace related stuff from the interface entirely. (If you post that in a reply I'll accept it.) – Ergwun Mar 21 '11 at 00:49

5 Answers5

8

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).

CraigM
  • 389
  • 2
  • 5
5

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...

sheikhjabootie
  • 7,308
  • 2
  • 35
  • 41
1

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 ;).

Somebody
  • 11
  • 1
1

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
}
Marius Bancila
  • 16,053
  • 9
  • 49
  • 91
1

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.");
    }
}
Can Gencer
  • 8,822
  • 5
  • 33
  • 52
  • 1
    I'm guessing the IFoo from the OP actually has some behaviours other than the DoIt method that he wants to remain on the interface. In which case, passing a "Null object" instead of the concrete class would lose some of the behaviour that he wanted to keep. – sheikhjabootie Mar 09 '11 at 10:09