How can I pass an object between functions

Question

This is driving me abit nuts so if anyone could hepl i'd be very grateful!!

I am trying to log my information to a log file so I used a Logger class.

** Logger.cs **

 public  class Logger : IDisposable
{
    private readonly FileStream _file; //Only this instance have a right to own it
    private readonly StreamWriter _writer;
    private readonly object _mutex; //Mutex for synchronizing

    /// <summary>
    /// Call this function to use the text file
    /// </summary>
    /// <param name="logPath"></param>
    public Logger(string logPath)
    {
        if (logPath != null) _file = new FileStream(logPath, FileMode.Append);
        _writer = new StreamWriter(_file);
        _mutex = new object();
    }

    // Log is thread safe, it can be called from many threads
    public void Log(string message)
    {
        lock (_mutex)
        {
           //_writer.Write("\r\nLog Entry : ");
           // _writer.WriteLine("{0} {1}", DateTime.Now.ToLongTimeString(),
           //DateTime.Now.ToLongDateString());

            _writer.WriteLine("{0} {1}", DateTime.Now.ToString("yyyy-MM-dd"),
          DateTime.Now.ToLongTimeString());
            _writer.WriteLine(message);
        }
    }

    /// <summary>
    /// Call this function when it says file is already been using somewhere
    /// </summary>
    public void Dispose()
    {
        _writer.Dispose(); //Will close underlying stream
    }
}

** My Application Using Logger Class **

private void button1_Click(object sender, EventArgs e)
    {
        // This is the file path after d://dashboardLogfiles
        String filePath = string.Format("{0:yyyy-MM-dd}", DateTime.Now);

        // This is the text file created with time stamps
        String txtFile = string.Format("DataSummarisation{0:yyyy-MM-dd hh-mm-ss-tt}", DateTime.Now);
        // Given in config to read the path 
        var localhostizedLetter = @"d:/";
        //Create directory
        string pathString = Path.Combine(localhostizedLetter, "DataSummarisationLogfiles");
        if (!Directory.Exists(pathString))
        {
            Directory.CreateDirectory(pathString);
        }
        // Create a folder inside directory 
        // If folder exists dont create it 
        pathString = Path.Combine(pathString, filePath);
        if (!Directory.Exists(pathString))
        {
            Directory.CreateDirectory(pathString);
        }

        // create a file inside d://DataSummarisationDatetime.now//datetimewithtimestamp.txt
        // if exists please dont create it.
        pathString = Path.Combine(pathString, txtFile);
        if (!Directory.Exists(pathString))
        {
            // here my file is created and opened.
            // so I m doing a try catch to make sure if file is opened we are closing it so that nother process can use it
            File.Create(pathString).Dispose();
            var fileInfo = new FileInfo(pathString);
           // IsFileLocked(fileInfo);

        }

        _logger = new Logger(pathString);
        _logger.Log("Log File Created");
        _logger.Dispose();
        ThreadStart starter = () => MigrateProductStats(123, 0, pathString);
        var thread = new Thread(starter);
        thread.Start();
    }

** My Functions Using Same Logger Path **

 private void MigrateProductStats(object corporationIdObj, object brandIdObj, object logFilePath)
    {
         _logger = new Logger(logFilePath.ToString());
        _logger.Log("Am I still writing to same file?");
        _logger.Dispose();
        for (int i = 0; i <= 10;i++ )
        {
            DoProductStatsForCorporation(123, logFilePath.ToString());
        }

    }
    private void DoProductStatsForCorporation(int corporationId, string logFilePath)
    {
        _logger = new Logger(logFilePath);
        _logger.Log("Am I still writing to same file second time?");
        _logger.Dispose();
    }

** Above Scenario is Working **

** But I am expecting to pass object instead of Path to avoid Reinstatiaitng **

  ThreadStart starter = () => MigrateProductStats(123, 0, _logger);
    var thread = new Thread(starter);
    thread.Start();

In the above case in my Button click I am disposing logger and sending the path to the functions DoProductStatsForCorporation and MigrateProductStats instead of that if I try sending _logger object without disposing that and avoiding to Reiniate in my child functions I am getting error cannot write to the file as it is used by another process.

I hope that makes sense!

Any guidance on this would be much appreciated as i am pretty stuck on where to go with this.

Answer 1

For starters DoProductStatsForCorporation logger should not be parameter of this function. Your logger should be created at start, and beeing a field in class.

You do not want to pass logger to functions because function don't really need this logger to do its job.

Move logger out of some button code, create it in constructor and use like from private field in class where logging takes place.

If you want to change log files in the same class and log to different files then you have to rewrite your logger class so it could use several files.

Also you can learn about Log4net it is really nice.

Answer 2

I'm slightly confused. Why do you pass the file path to the Logger? It should probably control this itself. Personally, I would make my Logger class static and then synchronize the methods like so:

[MethodImpl(MethodImplOptions.Synchronized)]
public static void Log(string description)

Then, use and dispose of the writer inline to avoid these very issues.

using (var writer = new StreamWriter(Path + "\\" + LOG_FILE, true))
{
    writer.WriteLine(log);
    writer.Close();
}

Answer 3

The problem you have is that it's MT and most likely you are probably writing to a file that is already open (race conditions)

why dont you use a singleton for your logger/writer? why dont you lock the writer and only ever use the one instance instead of always creating a new instance?

also your path string looks really wrong. I mean why spit out every last milisecond/tick in the file name?

I suggest you use the singleton approach for logging or if you must, create a static instance of the logger and lock the file when you are writing and dispose when complete. you are writing to the file that maybe in use whilst other threads are accessing that file. Be sure that the filename is also unique - it may not appear to be what you think it is in the code.