Locking mechanism in C#

I would like you to review the locking mechanism I implement in C# class. Will it be working correctly with many threads?
Please don't mind rest of the code because I just try to repair the class quickly to have a thread safe logging class for my project.
Any other suggestions are welcome.

```
public static class Debug
{

static readonly object _consoleLocker = new object();
static readonly object _diskDriveLocker = new object();

public static void Log(Tryb tryb, int level, string caller, string message, string messageExt)
{
String strAppDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().GetName().CodeBase);
strAppDir = strAppDir.Replace("file:\\", ""); // dla windowsa
strAppDir = strAppDir.Replace("file:", ""); // dla linuksa
string file = Path.Combine(strAppDir, "log.txt");
ProcessLogMessage(tryb, "[log]", caller, message, messageExt, file);
}

public static void Log(Tryb tryb, int level, string caller, string message, string messageExt, string file)
{
ProcessLogMessage(tryb, "[log]", caller, message, messageExt, file);
}

public static void Error(Tryb tryb, string caller, string message, string messageExt)
{
String strAppDir = Path.GetDirectoryName(Assembly.GetExecutingAssembly().GetName().CodeBase);
strAppDir = strAppDir.Replace("file:\\", ""); // dla windowsa
strAppDir = strAppDir.Replace("file:", ""); // dla linuksa
string file = Path.Combine(strAppDir, "error.txt");
ProcessLogMessage(tryb, "[error]", caller, message, messageExt, file);
}

public static void Error(Tryb tryb, string caller, string message, string messageExt, string file)
{
ProcessLogMessage(tryb, "[error]", caller, message, messageExt, file);
}

static void ProcessLogMessage(Tryb tryb, string rodzaj, string caller, string message, string messageExt, string file)
{
string dane = String.Format("{0}, {1}, {2}, {3},

First of all, your locking seems ok, I don't expect any drawbacks by using multiple threads.

That being said, let us review this from bottom to top.

public enum Tryb
{
    FILE = 0,
    CONSOLE = 1,
    FILEANDCONSOLE = 2
}

-
The name of that enum is at the nicest say unclear. A name like LogTarget would be more clear and its meaning would be obvious to any reader/user of this code.

-
Because it only has 3 members you should consider to add the [Flags] atribute to that enum which would result in a much shorter code, because you could omit the switch statement in ProcessLogMessage method.



This would look like so

[Flags]
public enum LogTarget
{
    FILE = 1,
    CONSOLE = 2,
    FILEANDCONSOLE = FILE | CONSOLE
}

static void WriteLogToFile(string strLogMessage, string strLogFile)
{
    StreamWriter swLog = null;

    try
    {
        if (!File.Exists(strLogFile))
            swLog = new StreamWriter(strLogFile);
        else
            swLog = File.AppendText(strLogFile);

        swLog.WriteLine(strLogMessage);
    }
    finally
    {
        if (swLog != null)
        {
            swLog.Close();
            swLog.Dispose();
        }
    }
}

-
instead of having a Try..Finally and 2 different ways of creating the StreamWriter you should use the overloaded constructor StreamWriter(string, boolean) and enclose its usage in a using block which takes care of disposing and closing automatically.

Based on the valid comment of @Heinzi


You don't need bool append = File.Exists(strLogFile);, just use new
StreamWriter(strLogFile, true). According to the documentation‌​, the
Boolean parameter has no effect if the file does not exist

you can just use true for append so no need to check if the file exists.

-
although braces {} are optional for single line if..else statements you really should use them because they will make your code less error prone.

Applying this will lead to

static void WriteLogToFile(string strLogMessage, string strLogFile)
{
    using (StreamWriter swLog = new StreamWriter(strLogFile, true))
    {
        swLog.WriteLine(strLogMessage);
    }
}

static void ProcessLogMessage(Tryb tryb, string rodzaj, string caller, string message, string messageExt, string file)
{
    string dane = String.Format("{0}, {1}, {2}, {3}, {4}", DateTime.Now, rodzaj, caller, message, messageExt);
    switch (tryb)
    {
        case Tryb.FILE:
            lock (_diskDriveLocker)
            {
                WriteLogToFile(dane, file);
            }
            break;
        case Tryb.CONSOLE:
            lock (_consoleLocker)
            {
                Console.WriteLine(dane);
            }
            break;
        case Tryb.FILEANDCONSOLE:
            lock (_consoleLocker)
            {
                Console.WriteLine(dane);
            }
            lock (_diskDriveLocker)
            {
                WriteLogToFile(dane, file);
            }
            break;
    }
}

Now we can use the enum by calling the HasFlags() method which results in only 2 if statements.

static void ProcessLogMessage(LogTarget target, string rodzaj, string caller, string message, string messageExt, string file)
{
    string logMessage = String.Format("{0}, {1}, {2}, {3}, {4}", DateTime.Now, rodzaj, caller, message, messageExt);

    if (target.HasFlag(LogTarget.FILE))
    {
        lock (_diskDriveLocker)
        {
            WriteLogToFile(logMessage, file);
        }
    }

    if (target.HasFlag(LogTarget.CONSOLE))
    {
        lock (_consoleLocker)
        {
            Console.WriteLine(logMessage);
        }
    }   
}

• The composition of the error.txt and log.txt filenames should be done in separate methods and be assigned to class level variables so they could be reused.

StackExchange Code Review Q#108244, answer score: 16