Writing to log file: using (... new StreamReader) every time I write a line?

Question

I have written an automation program with a start and a stop button, that does various tasks over and over, until user clicks stop. In the program I have several Console.WriteLines that shows the outcome of each little sub-task. I have decided to convert these to a .txt log, so i can review after shutting down the program, or in case it crashes.

I made the following Logger class:

class Logger
{
    private string _logName;

    public Logger(string logName)
    {
        _logName = logName;

        using (StreamWriter w = File.AppendText(_logName + ".txt"))
        {
            w.WriteLine("-------------------------------");
        }
    }

    public void Log(string logMessage)
    {
        using (StreamWriter w = File.AppendText(_logName + ".txt"))
        {
            w.WriteLine(DateTime.Now.ToLongDateString() + " " + DateTime.Now.ToLongTimeString() + " " + logMessage);
        }
        Console.WriteLine(logMessage);
    }
}

The question: In another class i have a Logger instance called _logger. I call _logger.Log over and over, each time I want to write a line to the log. Sometimes that goes very quickly after each other. It feels wrong to create a new instance of StreamWriter every time I write a line to the log (in the using statement). I therefore thought of putting the instantiation of StreamWriter in the constructor, but that breaks the cleanup features of the using statement, does it not?

Any idea how best to do this?

Thanks in advance.

Any reason for not using a [logging framework](http://stackoverflow.com/questions/185542/net-logging-framework)? — Sriram Sakthivel, Aug 21 '14 at 13:13

score 3 · Accepted Answer · edited May 23 '17 at 12:00

3

You could ensure yourself that the stream gets disposed/closed when you're finished (so when the instance of this class gets disposed):

public sealed class Logger : IDisposable
{
    private string _logName;
    private StreamWriter _stream;

    public Logger(string logName)
    {
        _logName = logName;
        _stream = File.AppendText(_logName + ".txt");
    }

    public void Log(string logMessage)
    {
        _stream.WriteLine(DateTime.Now.ToLongDateString() + " " + DateTime.Now.ToLongTimeString() + " " + logMessage);
    }

    public void Dispose()
    {
        _stream.Dispose();
    }
}

Now you could also use your logger with the using-statement:

using (Logger log = new Logger("Path"))
{
    log.Log("Something");
    // do something else...
    log.Log("Something else");
}

Or you could use an available logging library: .NET logging framework

edited May 23 '17 at 12:00

Community

1
1

answered Aug 21 '14 at 13:17

Tim Schmelter

450,073
74
686
939

2

Just as a "Best Practices" note, if you are not going to follow the `protected virtual void Dispose(bool Disposing)` pattern (and there is nothing wrong with doing that), you really should mark the class `sealed`. – Scott Chamberlain Aug 21 '14 at 13:21
Tim, this unfortunately still leaves me with the same problem i had before (just moved to the calling class). In that class (the one that sends the text to the logger) I only want to create one Logger instance. I do not want to create a Logger instance every time I write one simple line to the log file (which happens very frequently in succession). – Magnus Aug 21 '14 at 13:41
@Magnus: it's not clear why you cant control that only one instance of `Logger` is used. – Tim Schmelter Aug 21 '14 at 13:50
@TimSchmelter Post says using(Logger log = new Logger("Path")) { send_string_line_to_be_logged }. So every time i send a line, a new Logger instance (log) is created, right? – Magnus Aug 21 '14 at 13:54
@Magnus: yes, but in your post you can't ensure that the stream gets closed which is why you alwys use the `using`-statement in the class itself. Now you can use the `using` (or `try-finally`) outside which enables you to wrap the relevant part of the program into a `using`. If you have another main-class which controls everything you could hold the single instance there, either as field which you dispose in your own `Dispose` or as `using` around the main-code. – Tim Schmelter Aug 21 '14 at 14:10
@TimSchmelter Thanks Tim. So, in reality I only have three options: 1) Wrap my entire class into a using statement (I use the logger throughout the whole class). 2) In the parent class/thread, where an instance is created of the class that needs to update the log (call it Worker), I wrap the instance into using statement and pass log instance into the constructor: using (Logger log = new Logger("Path")) { Worker _worker = new Worker(log) }. 3) Use an external logging library like log4net. Is that right? If so, which method do you prefer? 1) seems messy to me. Does 2) hurt performance? Thanks! – Magnus Aug 21 '14 at 14:24

score 1 · Answer 2 · answered Aug 21 '14 at 13:19

You can make your logger class disposeable, now it is the responsibility of the caller to trigger the cleanup.

class Logger : IDisposeable
{
    private string _logName;
    private StreamWriter _w;

    public Logger(string logName)
    {
        _logName = logName;

        _w = File.AppendText(_logName + ".txt"))
        _w.WriteLine("-------------------------------");

    }

    public void Log(string logMessage)
    {
        _w.WriteLine(DateTime.Now.ToLongDateString() + " " + DateTime.Now.ToLongTimeString() + " " + logMessage);
        Console.WriteLine(logMessage);
    }

    public void Dispose()
    {
        Dispose(true);
    }

    protected virtual void Dispose(bool disposing)
    {
        if(disposing)
            _w.Dispose();
    }
}

However there are much more robust pre-made logging frameworks out there, I recommend you use one of those instead of writing your own.

Writing to log file: using (... new StreamReader) every time I write a line?

2 Answers2