4
\$\begingroup\$

I need to extend the default TextWriterTraceListener class to include timestamps with each message on same line and file rotation. I can't seem to find any way of doing it with TraceListener configuration.

Can you please review it and let me know any flaws to be used in a multi-threaded application?

using System;
using System.Collections.Generic;
using System.IO;
using System.Diagnostics;
using System.Text;
namespace ConsoleApplication1
{
    public class MyTextWriterTraceListener : TextWriterTraceListener
    {
        public MyTextWriterTraceListener(System.IO.Stream stream) : base(stream) { }
        public MyTextWriterTraceListener(System.IO.Stream stream, string name) : base(stream, name) { }
        public MyTextWriterTraceListener(string FileName, string name) : base(FileName, name) { RotateLogFiles(FileName); }
        public MyTextWriterTraceListener(string FileName) : base(FileName) { RotateLogFiles(FileName); }
        public MyTextWriterTraceListener(System.IO.TextWriter writer, string name) : base(writer, name) { }
        public MyTextWriterTraceListener(System.IO.TextWriter writer) : base(writer) { }
        public override void Write(string Msg)
        {
            base.Write(Msg);
            base.Flush();
        }
        public override void WriteLine(string Msg)
        {
            base.WriteLine(LinePrefix() + Msg);
            base.Flush();
        }

        private void RotateLogFiles(string FileName)
        {
            FileInfo TheFileInfo = new FileInfo(FileName);
            if (TheFileInfo.Exists == false)
                return;
            if (TheFileInfo.LastWriteTime.Date < DateTime.Today)
            {
                TheFileInfo.MoveTo(string.Format(@"{0}\{1}_{2}{}", TheFileInfo.DirectoryName, TheFileInfo.Name, TheFileInfo.LastWriteTime.ToString(), TheFileInfo.Extension));
            }
        }

        [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.NoInlining)]
        private string LinePrefix()
        {
            DateTime Now = DateTime.Now;
            return string.Format("{0} : ", Now.ToString("dd-MMM-yyyy HH:mm:ss"));
        }
    }
}
\$\endgroup\$
5
\$\begingroup\$

Possible issues

1 Follow a style guideline. It is common convention to camel case local variable names.

var theFile = new FileInfo(...) instead of var TheFile = new FileInfo(...).

Format your file equally (i.e. either all methods are separated by a blank line or none).

2 Keep lines short and use more variables to get better readability and easier debugging:

var newPath = Path.Combine(file.DirectoryName, file.Name, file.LastWriteTime.ToString(), file.Extension)
file.MoveTo(newPath);

instead of

TheFileInfo.MoveTo(string.Format(@"{0}\{1}_{2}{}", TheFileInfo.DirectoryName, TheFileInfo.Name, TheFileInfo.LastWriteTime.ToString(), TheFileInfo.Extension));

3 Use meaningful names.

RollingTextWriterTraceListener instead of MyTextWriterTraceListener

[MethodImpl(MethodImplOptions.NoInlining)] instead of [System[...]NoInlining)]

What do you do if you need an other implementation - MyOtherTextWriterTraceListener? The same applies to TheFileInfo, ConsoleApplication1.

4 Remove redundant qualifiers to remove clutter.

public MyTextWriterTraceListener(Stream stream) : base(stream) { }

instead of

public MyTextWriterTraceListener(System.IO.Stream stream) : base(stream) { }

var file = new FileInfo(...) instead FileInfo file = new FileInfo(...)

5 Are you sure you need the [MethodImpl(MethodImplOptions.NoInlining)] attribute?

What I like

1 if (TheFileInfo.Exists == false) return;

because the return statement reduces nesting (see arrow head anti pattern) and == false because its more obvious and than !TheFileInfo.Exists. A lonely ! can quick be overseen. I would recommend the same procedure for the following if statement, too:

if (TheFileInfo.LastWriteTime.Date >= DateTime.Today) return;

instead of

if (TheFileInfo.LastWriteTime.Date < DateTime.Today) { ... }

General recommendations

1 As k3b mentioned I would recommend using log4net, too. It has more flexibility you will ever need and you can configure it via config file which means you can change your logging output without recompiling.

\$\endgroup\$
2
\$\begingroup\$

In my opinion the code is ok.

there may be some minor issues

  • Rotate-logfile is only called in the constructor of MyTextWriterTraceListener. So if you are tracing long running applications (i.e. a service) the logfile is not rotated at all.
  • The LinePrefix with dateTime is only called in writeline. If you are constructing the line with several Write-statements the prefix might go to the wrong place.
  • the formatig of RotateLogFiles removes the fileextenstion string.Format(@"{0}\{1}_{2}{}" the last argument {} should be {3}. I prefer to use Path.Combine methods that also verifies validity of filenames instead of string.Format.

I personally prefer to use log4net for logging, that already supports your hommade features (DateTime-Prefix, FileName-Rotation)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.