patterncsharpMinor

Is calling file write in the dispose acceptable?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

filethewritedisposecallingacceptable

Problem

class HtmlLogger:ILogger,IDisposable 
{
    private System.IO.StreamWriter  _file;
    private bool _disposed;

    public HtmlLogger()
    {
        _disposed = false;
        _file = new StreamWriter(@"somepath");
        _file.Write("");
    }

    public void Log(string message)
    {
        _file.Write("{0}", message);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                if (_file != null)
                {
                    _file.Write("");
                    _file.Flush();
                    _file.Dispose();
                }

            }
            _file = null;
            _disposed = true;
        }
    }
}

Solution

I think it is good idea to use existing classes, I've already made some of the refacorings along with some design choices. For additional info, please read this book:

Design Patterns

EDIT: I think that explicit command calls (open log, write something and close it) before dispose the log object are much more readable, easy to understand and maintain later, than impementing the dispose pattern in another object (so you will get at least two objects to control lifecycle: custom class, and StreamWriter class - that will break the Einstein rule, - it's just not good to control 2 objects, when you could simply control one). In particular, that is going to be true when the new contracts will be applied later to the code. Disposable pattern is to heavy for such simple task. Additionally, as an advantage of using one object to control, and - consequently, - very simplified logic, it's easier to control log writer object's lifetime. At second, simple class with a good and clead design can be extended easily to supprt for an additional logic and do will not likely contains any of the caveats in the future.

public interface IHTMLLogger
{
    ILogWriter CreateHTMLLogWriter(string filePath);
}

public interface ILogWriter : IDisposable
{
    void OpenLog();
    void WriteLog(string message);
    void CloseLog();
}

public class HtmlLogger : IHTMLLogger
{
    public class HTMLLogWriter : StreamWriter, ILogWriter
    {
        public HTMLLogWriter(string filePath)
            : base(filePath)
        {
        }
        public void WriteLog(string message)
        {
            base.Write(string.Format("{0}", message));
        }
        public void OpenLog()
        {
            base.Write("");
        }
        public void CloseLog()
        {
            base.Write("");
        }
    }

    public ILogWriter CreateHTMLLogWriter(string filePath)
    {
        return new HTMLLogWriter(filePath);
    }
}

EDIT: As you see, I can easily integrate new encoding logic to encode any string data:

public interface IHTMLEncoder
{
    string Encode(string message);
}

public interface IHTMLLogger
{
    ILogWriter CreateHTMLLogWriter(string filePath);
}

public interface ILogWriter : IDisposable
{
    void OpenLog();
    void WriteLog(string message);
    void CloseLog();
}

public class HtmlLogger : IHTMLLogger
{
    public class HTMLEncoder : IHTMLEncoder
    {
        public string Encode(string message)
        {
            return Convert.ToBase64String(Encoding.UTF8.GetBytes(message.ToCharArray()));
        }
    }
    public class HTMLLogWriter : StreamWriter, ILogWriter
    {
        private IHTMLEncoder _encoder;
        public HTMLLogWriter(string filePath, IHTMLEncoder encoder)
            : base(filePath)
        {
            _encoder = encoder;
        }
        public void WriteLog(string message)
        {
            base.Write(string.Format("{0}", _encoder.Encode(message)));
        }
        public void OpenLog()
        {
            base.Write("");
        }
        public void CloseLog()
        {
            base.Write("");
        }
    }    
    public ILogWriter CreateHTMLLogWriter(string filePath)
    {
        return new HTMLLogWriter(filePath, new HTMLEncoder());
    }
}

Sample:

```
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Configuration;
using System.Runtime.Serialization;
using System.Security.Permissions;
using System.IO;
using System.Diagnostics;

namespace WindowsFormsApplication1
{
public partial class Form1 : Form
{
private static IHTMLLogger logger = new HtmlLogger();
private ILogWriter logWriter;

public Form1()
{
InitializeComponent();
}
private void Form1_Load(object sender, EventArgs e)
{
logWriter = logger.CreateHTMLLogWriter(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "html.log"));
logWriter.OpenLog();

this.Location = UserSettings.Default.FormLocation;
this.Size = UserSettings.Default.FormSize;
this.textBox1.Text = UserSettings.Default.ProcessPath;
}

private void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
UserSettings.Default.FormLocation = this.Location;
UserSettings.Default.FormSize = this.Size;
UserSettings.Default.ProcessPath = this.textBox1.Text;
UserSettings.Default.Save();

logWriter.CloseLog();
logWriter.Dispose();

MessageBox.Show(File.ReadAllText(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "html.log")));
}

private void textBox1_TextChanged(object sender, EventArgs e)
{
UserSettings.Default.ProcessPath = this.textBox1.Text;
logWriter.WriteLog(this.textBox1.Text);
}
}

Code Snippets

public interface IHTMLLogger
{
    ILogWriter CreateHTMLLogWriter(string filePath);
}

public interface ILogWriter : IDisposable
{
    void OpenLog();
    void WriteLog(string message);
    void CloseLog();
}

public class HtmlLogger : IHTMLLogger
{
    public class HTMLLogWriter : StreamWriter, ILogWriter
    {
        public HTMLLogWriter(string filePath)
            : base(filePath)
        {
        }
        public void WriteLog(string message)
        {
            base.Write(string.Format("<DIV>{0}</DIV>", message));
        }
        public void OpenLog()
        {
            base.Write("<HTML><BODY>");
        }
        public void CloseLog()
        {
            base.Write("</BODY></HTML>");
        }
    }

    public ILogWriter CreateHTMLLogWriter(string filePath)
    {
        return new HTMLLogWriter(filePath);
    }
}

public interface IHTMLEncoder
{
    string Encode(string message);
}

public interface IHTMLLogger
{
    ILogWriter CreateHTMLLogWriter(string filePath);
}

public interface ILogWriter : IDisposable
{
    void OpenLog();
    void WriteLog(string message);
    void CloseLog();
}

public class HtmlLogger : IHTMLLogger
{
    public class HTMLEncoder : IHTMLEncoder
    {
        public string Encode(string message)
        {
            return Convert.ToBase64String(Encoding.UTF8.GetBytes(message.ToCharArray()));
        }
    }
    public class HTMLLogWriter : StreamWriter, ILogWriter
    {
        private IHTMLEncoder _encoder;
        public HTMLLogWriter(string filePath, IHTMLEncoder encoder)
            : base(filePath)
        {
            _encoder = encoder;
        }
        public void WriteLog(string message)
        {
            base.Write(string.Format("<DIV>{0}</DIV>", _encoder.Encode(message)));
        }
        public void OpenLog()
        {
            base.Write("<HTML><BODY>");
        }
        public void CloseLog()
        {
            base.Write("</BODY></HTML>");
        }
    }    
    public ILogWriter CreateHTMLLogWriter(string filePath)
    {
        return new HTMLLogWriter(filePath, new HTMLEncoder());
    }
}

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Configuration;
using System.Runtime.Serialization;
using System.Security.Permissions;
using System.IO;
using System.Diagnostics;

namespace WindowsFormsApplication1
{
    public partial class Form1 : Form
    {
        private static IHTMLLogger logger = new HtmlLogger();
        private ILogWriter logWriter;

        public Form1()
        {
            InitializeComponent();
        }
        private void Form1_Load(object sender, EventArgs e)
        {
            logWriter = logger.CreateHTMLLogWriter(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "html.log"));
            logWriter.OpenLog();

            this.Location = UserSettings.Default.FormLocation;
            this.Size = UserSettings.Default.FormSize;
            this.textBox1.Text = UserSettings.Default.ProcessPath;
        }

        private void Form1_FormClosing(object sender, FormClosingEventArgs e)
        {
            UserSettings.Default.FormLocation = this.Location;
            UserSettings.Default.FormSize = this.Size;
            UserSettings.Default.ProcessPath = this.textBox1.Text;
            UserSettings.Default.Save();

            logWriter.CloseLog();
            logWriter.Dispose();

            MessageBox.Show(File.ReadAllText(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "html.log")));
        }

        private void textBox1_TextChanged(object sender, EventArgs e)
        {
            UserSettings.Default.ProcessPath = this.textBox1.Text;
            logWriter.WriteLog(this.textBox1.Text);
        }
    }

    [SettingsSerializeAs(SettingsSerializeAs.Xml)]
    [Serializable]
    public sealed class UserSettings : ApplicationSettingsBase
    {
        private const string FormLocationProperty = "FormLocation";
        private const string FormSizeProperty = "FormSize";
        private const string ProcessPathProperty = "ProcessPath";

        private UserSettings() { }

        private static UserSettings _defaultInstance = new UserSettings();

        public static UserSettings Default { get { return _defaultInstance; } }

        [UserScopedSetting()]
        [DefaultSettingValue("0, 0")]
        public Point FormLocation
        {
            get { return (Point)(this[FormLocationProperty]); }
            set { this[FormLocationProperty] = value; }
        }

        [UserScopedSetting()]
        [DefaultSettingValue("300, 300")]
        public Size FormSize
        {
            get { return (Size)this[FormSizeProperty]; }
            set { this[FormSizeProperty] = value; }
        }

        [UserScopedSetting]
        [DefaultSettingValue("")]
        public string ProcessPath
        {
            get { return (string)this[ProcessPathProperty]; }
            set { this[ProcessPathProperty] = value; }
        }
    }
}

Context

StackExchange Code Review Q#5119, answer score: 2

Revisions (0)

No revisions yet.