No need to check if event tracing is enabled before calling TraceEvent()

[antiduh]

The code is littered with statements like this:

         if (Log.SerialTrace(System.Diagnostics.TraceEventType.Verbose))
                Log.Serial.TraceEvent(System.Diagnostics.TraceEventType.Verbose, 0,
                    "{0}: SerialThread: DoWriteEvent: WriteFile({1}, {2}, {3}, ...) == {4}", m_Name,
                    m_ComPortHandle.DangerousGetHandle(), bufPtr, bufLen, result);

Which boils down to a ShouldTrace() call on the LogSource for Serial, then a TraceEvent call on that same source.

However, TraceEvent's documentation says this:

The TraceEvent method calls the ShouldTrace method of the SourceSwitch object returned by the Switch property. If ShouldTrace returns true, TraceEvent calls the corresponding TraceEvent method of each listener. Otherwise, TraceEvent returns without calling the listeners' methods.

If I'm reading that correctly, there's no point in calling ShouldTrace yourself, it's just going to do it for you. That block above can be simplified to:

                Log.Serial.TraceEvent( TraceEventType.Verbose, 0,
                    "{0}: SerialThread: DoWriteEvent: WriteFile({1}, {2}, {3}, ...) == {4}", m_Name,
                    m_ComPortHandle.DangerousGetHandle(), bufPtr, bufLen, result);

Even if I'm wrong there's no reason why you couldn't use your Log wrapper class to do the same for you, instead of repeating the construct a hundred times.

[jcurl]

There is a problem in your logic, and the "if" statement is there to prevent boxing, because the compiler will box the parameters first, before knowing if it should trace or not. By having the "if" statement prior, we can avoid the boxing.

[antiduh]

That's a good point, I hadn't considered the boxing problem.

You can still avoid boxing if you use a small wrapper class with generic arguments, below. This example also calls string.Format() directly. string.Format() has overloads to accept up to 3 arguments before resorting to passing args via object[]. This avoids the creation of that object[] when tracing is enabled.

Cost/benefit:

• Cost: you have to have a class that takes generic arguments, and will need an arbitrary number of arguments.
• Benefit: logging callsites are simplified, improving the readability of your code.
• Benefit: overall LOC goes down.

        public class Logger
        {
            private TraceSource source;

            public Logger( TraceSource source )
            {
                this.source = source;
            }

            public void Log<A>( TraceEventType eventType, int id, string format, A arg0 )
            {
                if( source.Switch.ShouldTrace( eventType ) )
                {
                    source.TraceEvent( eventType, id, string.Format( format, arg0 ) );
                }
            }

            public void Log<A, B>( TraceEventType eventType, int id, string format, A arg0, B arg1 )
            {
                if( source.Switch.ShouldTrace( eventType ) )
                {
                    source.TraceEvent( eventType, id, string.Format( format, arg0, arg1 ) );
                }
            }

            public void Log<A, B, C>( TraceEventType eventType, int id, string format, A arg0, B arg1, C arg2 )
            {
                if( source.Switch.ShouldTrace( eventType ) )
                {
                    source.TraceEvent( eventType, id, string.Format( format, arg0, arg1, arg2 ) );
                }
            }

            public void Log<A, B, C, D>( TraceEventType eventType, int id, string format, A arg0, B arg1, C arg2, D arg3 )
            {
                if( source.Switch.ShouldTrace( eventType ) )
                {
                    source.TraceEvent( eventType, id, string.Format( format, arg0, arg1, arg2, arg3 ) );
                }
            }
        }