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);
}
}