Skip to main content
added 1053 characters in body
Source Link
Abbas
  • 5.6k
  • 24
  • 40

And now that you have the WriteError method, even your Log method can use it:

public static void Log(Exception ex)
{
    var sb = new StringBuilder();
    sb.WriteLine(DateTime.Now.ToString());
    sb.WriteLine($"Source {ex.Source.ToString().Trim()}");
    sb.WriteLine($"Message : {ex.Message.ToString().Trim()}");
    sb.WriteLine($"Inner Exceptions: {Convert.ToString(ex.InnerException)}");
    sb.WriteLine($"Exception thrown from: {GetExceptionGeneratedMethod(ex)}");
    sb.WriteLine($"Line Number: {GetExceptionGeneratedLineNumber(ex)}");
    sb.WriteLine($"Stack Trace: {ex.StackTrace.ToString()}");

    var path = AppDomain.CurrentDomain.BaseDirectory + "\\Log.txt";
    WriteError(path, sb.ToString());
}

You'll notice that I use this in the code: $"Source {ex.Source.ToString()}". In case you don't know, it's called String interpolation. You can read more about it here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/interpolated.

And now that you have the WriteError method, even your Log method can use it:

public static void Log(Exception ex)
{
    var sb = new StringBuilder();
    sb.WriteLine(DateTime.Now.ToString());
    sb.WriteLine($"Source {ex.Source.ToString().Trim()}");
    sb.WriteLine($"Message : {ex.Message.ToString().Trim()}");
    sb.WriteLine($"Inner Exceptions: {Convert.ToString(ex.InnerException)}");
    sb.WriteLine($"Exception thrown from: {GetExceptionGeneratedMethod(ex)}");
    sb.WriteLine($"Line Number: {GetExceptionGeneratedLineNumber(ex)}");
    sb.WriteLine($"Stack Trace: {ex.StackTrace.ToString()}");

    var path = AppDomain.CurrentDomain.BaseDirectory + "\\Log.txt";
    WriteError(path, sb.ToString());
}

You'll notice that I use this in the code: $"Source {ex.Source.ToString()}". In case you don't know, it's called String interpolation. You can read more about it here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/tokens/interpolated.

Source Link
Abbas
  • 5.6k
  • 24
  • 40

Coding style

Variable names

Avoid variable names like sw or st. In your code they still make sense because they're the abbreviation of the class name and it's not a lot of code, but it is still bad practice. When you have lots of code with less obvious class names or origins for the abbreviations, it'll become unclear what they are really fast. Rename them to streamWriter and trace or something similar that clearly indicates what it is.

Casing

Per the Naming Guidelines by Microsoft, use the following rules:

  • PascalCasing for methods, class names, ...
  • camelCasing for fields, parameters, ...

Your method names will become: WriteCustomErrorLog, GetExceptionGeneratedLineNumber, ...


Correctly rethrowing an Exception

Everywhere in your catch block you are rethrowing the exception with throw ex. This is bad practice since you'll lose the stacktrace. Just use throw to rethrow the exception. Implement it this way:

try
{
    //try something that might fail
}
catch (Exception ex)
{
    //Do something with the exception

    //Rethrow the exception
    throw;
}

More reading on this:


Disposing of the StreamWriter

Since the StreamWriter class derives from the TextWriter class and the latter implements the IDisposable interface, you can implement a using statement instead of trying to dispose of the object yourself:

using(var streamWriter = new StreamWriter("C:\\PATH_TO_FILE\\Log.txt", true))
{
    streamWriter.WriteLine("THE MESSAGE TO LOG");
}

StringBuilder instead of string concatenation

The quickest way of adding strings together is pasting them together with a + sign. Quicker isn't always the better practice, as is in this case. The better practice to combine strings is to use the StringBuilder.

Instead of something like this:

string result = "";
for(var i = 0; i < 100; i++)
{
    result += "SomeString";
}

You get this:

StringBuilder builder = new StringBuilder();
for(var i = 0; i < 100; i++)
{
    builder.Write("SomeString");
}
string result = builder.ToString();

Now in case, where you're not combining a lot of variables it might be better to use the String.Concat method instead of the Stringbuilder. And that code will look like following:

string firstName = "John";
string lastName = "Doe";
string fullName = String.Concat(firstname, " ", lastName);

Jon Skeet has an interesting article about this: http://jonskeet.uk/csharp/stringbuilder.html


Avoiding double code

Both variations of the writeCustomErrorLog method have almost the exact same logic in them. A better practice would be to extract the common logic in a separate method and call that method from the variations. This way you avoid a lot of work when expanding it or making changes to the implementation. Here's how you can do it:

public static void WriteCustomErrorLog(string message)
{
    var path = AppDomain.CurrentDomain.BaseDirectory + "\\PassiveLog.txt";
    WriteError(path, message);
}

public static void WriteCustomErrorLog(string location, string message,string fileName)
{
    var path = String.Format("{0}\\{1}.txt", location, fileName);
    WriteError(path, message)
}

private static void WriteError(string filePath, string message)
{
    using (var sw = new StreamWriter(filePath, true))
    {
        sw.WriteLine(message);
    }
}