Error-Handling Class and Logging I am relatively new to C# and would like to see if what I have been using for my exception handling, message and logging is along industry standards. I have created a simple class for the Error Handler and for the logging I use Log4Net. After reading Clean Code, I was thinking about separating DisplayMessage into 2 different procedures for the message and logging. The log file is usually written to a file server so multiple users can access it; I have enabled MinimalLock for this purpose. For my example, I have changed the log path to C:\Temp\. An example of a project I use the Error Handler class and logging in is on GitHub. For reference, I am using Visual Studio 2017 Community.



ErrorHandler.cs

using System;
using System.Windows.Forms;
using log4net;

[assembly: log4net.Config.XmlConfigurator(Watch = true)]

/// &lt;summary&gt; 
/// Used to handle exceptions
/// &lt;/summary&gt;
public class ErrorHandler
{
    private static readonly ILog log = LogManager.GetLogger(typeof(ErrorHandler));

    /// &lt;summary&gt; 
    /// Used to produce an error message and create a log record
    /// &lt;example&gt;
    /// &lt;code lang="C#"&gt;
    /// ErrorHandler.DisplayMessage(ex);
    /// &lt;/code&gt;
    /// &lt;/example&gt; 
    /// &lt;/summary&gt;
    /// &lt;param name="ex"&gt;Represents errors that occur during application execution.&lt;/param&gt;
    /// &lt;remarks&gt;&lt;/remarks&gt;
    public static void DisplayMessage(Exception ex)
    {
        System.Diagnostics.StackFrame sf = new System.Diagnostics.StackFrame(1);
        System.Reflection.MethodBase caller = sf.GetMethod();
        string currentProcedure = (caller.Name).Trim();
        string errorMessageDescription = ex.ToString();
        errorMessageDescription = System.Text.RegularExpressions.Regex.Replace(errorMessageDescription, @"\r\n+", " "); //the carriage returns were messing up my log file
        string msg = "Contact your system administrator. A record has been created in the log file." + Environment.NewLine;
        msg += "Procedure: " + currentProcedure + Environment.NewLine;
        msg += "Description: " + ex.ToString() + Environment.NewLine;
        log.Error("[PROCEDURE]=|" + currentProcedure + "|[USER NAME]=|" + Environment.UserName + "|[MACHINE NAME]=|" + Environment.MachineName + "|[DESCRIPTION]=|" + errorMessageDescription);
        MessageBox.Show(msg, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }

}


Usage Example:

/// &lt;summary&gt; 
/// Return the count of items in a delimited list
/// &lt;/summary&gt;
/// &lt;param name="valueList"&gt;Represents the list of values in a string &lt;/param&gt;
/// &lt;param name="delimiter"&gt;Represents the list delimiter &lt;/param&gt;
/// &lt;returns&gt;the number of values in a delimited string&lt;/returns&gt;
public int GetListItemCount(string valueList, string delimiter)
{
    try
    {
        string[] comboList = valueList.Split((delimiter).ToCharArray());
        return comboList.GetUpperBound(0) + 1;

    }
    catch (Exception ex)
    {
        ErrorHandler.DisplayMessage(ex);
        return 0;
    }
}




Log4Net



app.config

&lt;log4net&gt;
    &lt;appender name="ConsoleAppender" type="log4net.Appender.ConsoleAppender"&gt;
        &lt;layout type="log4net.Layout.PatternLayout"&gt;
            &lt;conversionPattern value="%date [%thread] %-5level %logger [%ndc] - %message%newline"/&gt;
        &lt;/layout&gt;
    &lt;/appender&gt;
    &lt;appender name="FileAppender" type="log4net.Appender.FileAppender"&gt;
        &lt;file value="C:\Temp\MyLogFile.log"/&gt;
        &lt;appendToFile value="true"/&gt;
        &lt;lockingModel type="log4net.Appender.FileAppender+MinimalLock"/&gt;
        &lt;layout type="log4net.Layout.PatternLayout"&gt;
            &lt;conversionPattern value="%date|%-5level|%message%newline"/&gt;
        &lt;/layout&gt;
    &lt;/appender&gt;
    &lt;root&gt;
        &lt;level value="ALL"/&gt;
        &lt;appender-ref ref="FileAppender"/&gt;
    &lt;/root&gt;
&lt;/log4net&gt;
I'm not familiar with log4net, so I will focus on the code itself.

Here is a list of what could be improved:


--- Style ---
ErrorHandler should be a static class to prevent newing up an instance of it
Lack of using namespace: Generally, you should declare all of them, unless:


there is an conflic in which case you can alias it: using A = Some.Namespace.A
a namespace is particularly noisy and is polluting the intellisense

Lack of var: Using the type name or the full name can make the code difficult to scan through. It is more difficult to locate a variable when they are not aligned. For the most part, the right hand side assignment should give enough of hint.

System.Diagnostics.StackFrame sf = new System.Diagnostics.StackFrame(1);
System.Reflection.MethodBase caller = sf.GetMethod();
string currentProcedure = (caller.Name).Trim();
// --- for comparison
var sf = new System.Diagnostics.StackFrame(1);
var caller = sf.GetMethod();
var currentProcedure = (caller.Name).Trim();

Lack of "spacing": Nobody likes reading a wall of text, and a wall of code is no better. You can divide the logical blocks of your code with some empty lines: (Think of them as paragraphs)

// gather context
var sf = new System.Diagnostics.StackFrame(1);
var caller = sf.GetMethod();

// format message
var currentProcedure = (caller.Name).Trim();
var errorMessageDescription = ex.ToString();
errorMessageDescription = System.Text.RegularExpressions.Regex.Replace(errorMessageDescription, @"\r\n+", " "); //the carriage returns were messing up my log file
var msg = "Contact your system administrator. A record has been created in the log file." + Environment.NewLine;
msg += "Procedure: " + currentProcedure + Environment.NewLine;
msg += "Description: " + ex.ToString() + Environment.NewLine;

// handle exception
log.Error("[PROCEDURE]=|" + currentProcedure + "|[USER NAME]=|" + Environment.UserName + "|[MACHINE NAME]=|" + Environment.MachineName + "|[DESCRIPTION]=|" + errorMessageDescription);
MessageBox.Show(msg, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);

Use of unnecessary parenthesis that doesn't highlight order of operation:


string currentProcedure = (caller.Name).Trim();
string[] comboList = valueList.Split((delimiter).ToCharArray());


--- code ---


string[] comboList = valueList.Split((delimiter).ToCharArray());
return comboList.GetUpperBound(0) + 1;



You can simply return comboList.Length here.


 /// &lt;param name="delimiter"&gt;Represents the list delimiter &lt;/param&gt;
 string[] comboList = valueList.Split((delimiter).ToCharArray());



You are using it as multiple delimiters, and not just one. The name should reflect that.


 /// &lt;returns&gt;the number of values in a delimited string&lt;/returns&gt;



The return of value 0 in error handling should be documented in the &lt;returns&gt; or &lt;remarks&gt; tag.




public static class ErrorHandler
{
    public static void DisplayMessage(Exception ex)
    {
        var sf = new System.Diagnostics.StackFrame(1);
        var caller = sf.GetMethod();
        var currentProcedure = caller.Name.Trim();

        var logMessage = string.Concat(new Dictionary&lt;string, string&gt;
        {
            ["PROCEDURE"] = currentProcedure,
            ["USER NAME"] = Environment.UserName,
            ["MACHINE NAME"] = Environment.MachineName,
            ["DESCRIPTION"] = ex.ToString().Replace("\r\n", " "), // the carriage returns were messing up my log file
        }.Select(x =&gt; $"[{x.Key}]=|{x.Value}|"));
        log.Error(logMessage);

        // pick one:
        var userMessage = new StringBuilder()
            .AppendLine("Contact your system administrator. A record has been created in the log file.")
            .AppendLine("Procedure: " + currentProcedure)
            .AppendLine("Description: " + ex.ToString())
            .ToString();
        var userMessage = string.Join(Environment.NewLine,
            "Contact your system administrator. A record has been created in the log file.",
            "Procedure: " + currentProcedure,
            "Description: " + ex.ToString(),
        ) + Environment.NewLine;
        MessageBox.Show(userMessage, "Unexpected Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}