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

ErrorManager management

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

Problem

Recently I created a class that would manager errors, log them, and write details about them to files when the function logError was called. It would be done by having an error code and error message and then displaying the error to the console and writing it to the file if the user wanted to.

I just posted this here to see if there was any way I can improve the class.

```
using log4net;
using System;
using System.Collections.Generic;

namespace Kiwi.Application.Base.Error
{
sealed class ErrorManager
{
private readonly Dictionary errorCodes;
private readonly ILog myLogger;

public ErrorManager()
{
errorCodes = new Dictionary();
myLogger = LogManager.GetLogger(typeof(ErrorManager));

// Add some errors and their messages to the dictionary
errorCodes.Add("bn1x", "Error when beginning to listen on server socket.");
errorCodes.Add("dm9e", "Unable to locate the error log file.");
}

public void logError(string errorCode, bool writeErrorToFile = false)
{
string errorMessage;
if (errorCodes.TryGetValue(errorCode, out errorMessage))
{
myLogger.Warn(errorMessage);

if (writeErrorToFile)
logToFile(errorCode);
}
else
{
logUnhandeldError("[Error code " + errorCode + "] " + errorCode);
}
}

private void logUnhandeldError(string errorCode)
{
myLogger.Error("Unhandeld error " + errorCode);
}

private void logToFile(string errorCode = "", string fileName = "logs/error.log")
{
try
{
using (System.IO.StreamWriter file = new System.IO.StreamWriter(fileName, true))
{
file.WriteLine("Error " + errorCode + " logged at " + getErrorTimestamp());
file.WriteLine("

Solution

Generally in .NET (especially C#) public members are PascalCase.

On a more important note: don't use strings for the errorCode. Instead, use an enum.

public enum ErrorCode
{
    ServerSocketBeginListeningFailure, // Rename this to something shorter if desired.
    LogFileNotFound,
}


You could also, potentially, make this a Flags enum, which would mean that error codes could be easily combined and used together to indicate multiple errors occurring at a time.

This will allow you to make more certain that the user doesn't specify a bad error code.

Use braces even when not necessary, they won't generally prevent bugs, but they will help prevent bugs.

Just as well, do not inline an if/else if/else statement when not using braces. Typically, when I see one of those, my eyes immediately follow the next line. I didn't notice return "" on your else in tryGetErrorMessage until I realized there was not a statement below it.

Code Snippets

public enum ErrorCode
{
    ServerSocketBeginListeningFailure, // Rename this to something shorter if desired.
    LogFileNotFound,
}

Context

StackExchange Code Review Q#113064, answer score: 4

Revisions (0)

No revisions yet.