HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

Console printing and logging method for a common library

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loggingmethodprintingforlibraryandconsolecommon

Problem

I have the following method that's in a common library. I want to know how to write it better, but mostly, I want to know how to think about this better so I don't end up with code like this.

```
public virtual void PrintConsoleAndLog(string verboseMessage = null, Exception e = null,
OnErrorEventsArgs.ShowExceptionLevel exceptionLevel =
OnErrorEventsArgs.ShowExceptionLevel.Error)
{
switch (exceptionLevel)
{
case OnErrorEventsArgs.ShowExceptionLevel.Info:
if (e == null)
{
Logger.Info("\n[{0}] {1}", DateTime.Now.ToShortTimeString(), verboseMessage);
Console.WriteLine("\n[{0}] {1}", DateTime.Now.ToShortTimeString(), verboseMessage);
}
else
{
if (verboseMessage == null)
{
Logger.Info(String.Format("\n[{0}]Base Exception: {1}\n\nStacktrace: {2}",
DateTime.Now.ToShortTimeString(),
e.GetBaseException(), e.StackTrace));

Console.WriteLine("[{0}]Base Exception: {1}\n\nStacktrace: {2}",
DateTime.Now.ToShortTimeString(),
e.GetBaseException(), e.StackTrace);
}
else
{
Logger.Info(String.Format("\n[{0}] Error: {1}\n\nBase Exception: {2}\n\nStacktrace: {3}",
DateTime.Now.ToShortTimeString(), verboseMessage,
e.GetBaseException(), e.StackTrace));

Console.WriteLine("[{0}] Error: {1}\n\nBase Exception: {2}\n\nStacktrace: {3}",
DateTime.Now.ToShortTimeString(), verboseMessage,
e.GetBaseExcepti

Solution

In the comments it was already mentioned to configure the logger to determine the logging target which avoids logging everything twice.

Another thing to improve: Eliminate duplicate code (statements and expressions).
From looking at the code there are many things which are repeated over and over again

  • Remove the time stamps. NLog can do that for you.



  • Especially ShortTimeString usually omits the seconds (at least in most standard locales I have seen). Not sure what kind of application you have but for error logging I would assume that it is usually better to have the timestamp as accurate as possible.



  • You log basically three types of messages:



  • Something when you do not have an exception object



  • Base Exception and Stacktrace



  • Base Exception and Stacktrace with extra error message



So: build them first and then simply decide the logging level. Something like this:

private const string NOMESSAGE = "";

public virtual void PrintConsoleAndLog(string verboseMessage = null, Exception e = null,
                                    OnErrorEventsArgs.ShowExceptionLevel exceptionLevel =
                                        OnErrorEventsArgs.ShowExceptionLevel.Error)
{
    var message = verboseMessage ?? NOMESSAGE;
    if (e != null)
    {
         var additionalMessage = GetAdditionalFormattedMessage(verboseMessage);
         message = string.Format("{0}Base Exception: {1}\n\nStacktrace: {2}", 
                            additionalMessage, e.GetBaseException(), e.StackTrace);
    }
    switch (exceptionLevel)
    {
        case OnErrorEventsArgs.ShowExceptionLevel.Info:
             Logger.Info(message);
             break;
        case OnErrorEventsArgs.ShowExceptionLevel.Debug:
             Logger.Debug(message);
             break;
        case OnErrorEventsArgs.ShowExceptionLevel.Error:
        default:
             Logger.Error(message);
             break;
    }
}

private string GetAdditionalFormattedMessage(string verboseMessage)
{
    return verboseMessage != null ? string.Format("Error: {0}\n\n", verboseMessage) : "";
}


Looks slightly cleaner to me. If you require the additional timestamp then it should be easy enough to add.

Update: Incorporated suggestions from comments.

Code Snippets

private const string NOMESSAGE = "<NOMESSAGE>";

public virtual void PrintConsoleAndLog(string verboseMessage = null, Exception e = null,
                                    OnErrorEventsArgs.ShowExceptionLevel exceptionLevel =
                                        OnErrorEventsArgs.ShowExceptionLevel.Error)
{
    var message = verboseMessage ?? NOMESSAGE;
    if (e != null)
    {
         var additionalMessage = GetAdditionalFormattedMessage(verboseMessage);
         message = string.Format("{0}Base Exception: {1}\n\nStacktrace: {2}", 
                            additionalMessage, e.GetBaseException(), e.StackTrace);
    }
    switch (exceptionLevel)
    {
        case OnErrorEventsArgs.ShowExceptionLevel.Info:
             Logger.Info(message);
             break;
        case OnErrorEventsArgs.ShowExceptionLevel.Debug:
             Logger.Debug(message);
             break;
        case OnErrorEventsArgs.ShowExceptionLevel.Error:
        default:
             Logger.Error(message);
             break;
    }
}

private string GetAdditionalFormattedMessage(string verboseMessage)
{
    return verboseMessage != null ? string.Format("Error: {0}\n\n", verboseMessage) : "";
}

Context

StackExchange Code Review Q#32218, answer score: 6

Revisions (0)

No revisions yet.