C# refactor if-else statement code

Question

Please check out my following code...

public enum LogType
{
    Debug,
    Info,
    Warn,
    Error,
    Fatal
}

private static readonly ILog log = 
log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);

public void LogError(LogType logtype, string message)
{
    XmlConfigurator.Configure();
    if (logtype == LogType.Debug)
        log.Debug(message);
    else if (logtype == LogType.Error)
        log.Error(message);
}

I do not like all the above if-else statements and believe that there is a cleaner way to write this. How could I refactor it? log class has different methods for Debug, Error etc. etc.

I would like to make a single call to a method have it automatically take care of it.

LogMyError(LogType.Debug, "I am just logging here");

How can I do such a thing? I prefer to stay away from switch statement. I am looking for a clean object oriented approach.

Answer 1

Your code is perfectly fine as it is; I would not change it.

However, it is instructive to think about how you would do this if you wanted to be more "object oriented" about it. Let's just consider two of your cases; the others you can easily see how they would be implemented:

public abstract class LogType
{
    public static readonly LogType Debug = new LogTypeDebug();
    public static readonly LogType Error = new LogTypeError();

    private LogType() {} // Prevent anyone else from making one.

    public abstract void LogMessage(ILog logger, string message);

    private sealed class LogTypeDebug: LogType
    {
        public override void LogMessage(ILog logger, string message)
        {
            logger.Debug(message);
        }
    }

    private sealed class LogTypeError: LogType
    {
        public override void LogMessage(ILog logger, string message)
        {
            logger.Error(message);
        }
    }
}
...

//Obtain the log object the way you prefer.
private static readonly ILog log = log4net.LogManager.GetLogger(System.Reflection.MethodBase.GetCurrentMethod().DeclaringType);

public void LogError(LogType logtype, string message)
{
    logtype.LogMessage(log, message);
}

The call site does not change at all! It still looks like:

LogError(LogType.Debug, "my message");

And there you go: no if or switch statements at all! The "switch on the type" code has been moved into the virtual function table, which is where it belongs in object-oriented code.

A nice side effect of this technique is you never need to worry about someone casting an integer to an unsupported value of your enumerated type. The only possible values for a variable of type LogType are null or a reference to one of the singletons.

Answer 2

You can use a Dictionary<LogType,Action<string>> to hold what action to perform for each enumeration value, then just call the delegate.

var logActions = new Dictionary<LogType,Action<string>>();
logActions.Add(LogType.Debug, log.Debug);
...

logActions[logtype](message);

---

Update:

This is all overkill if you do only have a small number of branches on your if statements. I would use this method for 5+ such ifs.