5

Generally, "using" is preferred approach for accessing and disposing of a filestream properly.

I often need to leave the file open (as below). Can the "using" structure be employed in this case?

public class logger
{
    private StreamWriter sw;
    public logger(string fileName)
    {
        sw = new StreamWriter(fileName, true);
    }

    public void LogString(string txt)
    {
        sw.WriteLine(txt);
        sw.Flush();
    }

    public void Close()
    {
        sw.Close();
    }
}
ManInMoon
  • 6,795
  • 15
  • 70
  • 133
  • 11
    There's no point in using `using` inside a method if you don't intend to close the stream within that scope. The general pattern here would be to make your `logger` implement [`IDisposable`](http://msdn.microsoft.com/en-us/library/system.idisposable(v=vs.110).aspx), then you basically make the calling class worry about when it gets disposed (using `using` or not) – p.s.w.g Nov 05 '14 at 16:08
  • 2
    Use NLog or another framework.# – Mike Miller Nov 05 '14 at 16:10
  • @ManInMoon how would you like to use *using* in this case, in pseudo code? – galenus Nov 05 '14 at 16:13
  • 1
    Could you explain why do you need to keep a file open between calls to LogString? For Performance reasons? Could you share your benchmarks that show how much did you gain keeping the file open vs open/close at every call to LogString? – Steve Nov 05 '14 at 16:13
  • @Steve: I've encountered situations in code which did a lot of logging where opening and closing the log on every operation meant that the *majority of the entire program's time* was spent opening and closing the log file, and where leaving the log open eased that. – supercat Nov 05 '14 at 16:16
  • @supercat the point is, this seems to be an optimization with a lot of potential disastrous consequences. Failing to close the stream... exception handling etc... as usual you need to know the context and measure. Albeit implementig IDisposable could be a safenet. – Steve Nov 05 '14 at 16:21
  • @Steve: Processes can be made almost arbitrarily slow if one tries to log everything in detail, and opens/closes the log file with every operation. It will thus usually be necessary to "optimize", either by limiting the amount of information one logs, or by improving logging speed; preparing for optimizations which will most likely be required is not "premature". Since improving logging speed will reduce the extent to which one must refrain from including information which could potentially be useful but "probably" won't be, I would favor optimization from that angle. – supercat Nov 05 '14 at 16:43

1 Answers1

5

Yes, you make Logger disposable and have it dispose of the stream in its dispose method.

// I make it sealed so you can use the "easier" dispose pattern, if it is not sealed
// you should create a `protected virtual void Dispose(bool disposing)` method.
public sealed class logger : IDisposable
{
    private StreamWriter sw;
    public logger(string fileName)
    {
        sw = new StreamWriter(fileName, true);
    }

    public void LogString(string txt)
    {
        sw.WriteLine(txt);
        sw.Flush();
    }

    public void Close()
    {
        sw.Close();
    }

    public void Dispose()
    {
        if(sw != null)
            sw.Dispose();
    }
}
Mike Miller
  • 16,195
  • 1
  • 20
  • 27
Scott Chamberlain
  • 124,994
  • 33
  • 282
  • 431
  • 1
    In void Dispose() did you mean to call sw.Close() or is sw.Dispose() correct – ManInMoon Nov 05 '14 at 16:15
  • 3
    @ManInMoon `Dispose` will close the stream - it's an implementation detail but is documented. – D Stanley Nov 05 '14 at 16:20
  • There is also a good read on SO on how to [Property use the IDisposable interface](http://stackoverflow.com/questions/538060/proper-use-of-the-idisposable-interface) with additional information about the interface. – Erik Philips Nov 07 '14 at 21:27