C#重构if-else语句代码

Question

请查看我以下的代码...

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

我不喜欢所有的if-else语句，认为有更加简洁的方式来编写代码。如何重构它呢？log类有不同的方法，例如Debug、Error等等。

我想只需要调用一个方法，就可以自动处理这些问题。

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

我该如何做到这一点？我更喜欢避免使用 switch 语句。我正在寻找一种干净的面向对象方法。

- dotnet-practitioner

---

这不是你已经在做的吗？ - Matthew

除了 switch 块之外，如果只有两个选项可供选择，我不会花太多时间考虑重构模式。这种方式非常易读。 - Alex

我也不认为有任何改变的理由 - 对于重构或任何设计改进，通常你必须有一个原因，即你试图“解决”或改进什么，使其更简单等。我在这里看不到这一点，一个简单的方法已经可以胜任，如果有更多“类型”，则使用switch。 - NSGaga-mostly-inactive

Answer 1

你的代码已经完全正确，我不会对它进行改变。

然而，思考一下如果你想更加“面向对象”地实现它该怎么做是有益的。让我们只考虑你的两种情况；其他情况你可以很容易地看出它们如何实现：

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

调用位置完全没有改变！它看起来仍然是这样：

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

就这样：不需要任何 if 或 switch 语句！“在类型上使用 switch”代码已经被移动到虚函数表中，这正是面向对象代码中它所属的地方。

这种技术的一个好处是，您永远不需要担心有人将整数强制转换为枚举类型的不支持值。类型为 LogType 的变量的唯一可能值为 null 或单例之一的引用。

- Eric Lippert

---

如果我有多个要切换的 Action<T>，那么这绝对是我会采取的方法。 - Oded

Eric，看起来我的GetLogger和你的GetLogger不一样。 - dotnet-practitioner

@dotnet-practitioner：这和问题有什么相关性呢？我不明白你的思路。回答的重点是向您展示如何以面向对象的方式完成此任务；它有什么不清楚的地方吗？ - Eric Lippert

@EricLippert 我认为OP是一个初学者，所以他没有意识到在你提供的代码示例中，实际获得日志对象的方式并不重要。我已经修改了您的代码示例，以便他更容易地跟随代码，希望这样做没问题。 - SolutionYogi

撇开避免使用“if/else”结构的技巧不谈，我认为OP的方法不正确。ILog接口定义了Info、Debug、Warn等方法。我认为OP应该直接在代码中使用这些方法，而不是创建另一个包装方法。例如，假设您要从FTP服务器下载文件。下载文件后，您可以使用Info()方法记录成功。如果发生异常，您可以使用Error方法（带有异常）记录错误。 - SolutionYogi

Answer 2

你可以使用一个Dictionary<LogType,Action<string>>，来保存每个枚举值对应的操作，然后只需调用委托即可。

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

logActions[logtype](message);

---

更新:

如果你只有很少的if语句分支，那么这都是过度设计。对于5个或更多的if语句分支，我会使用此方法。

- Oded

---

Oded，谢谢。如果可能的话，我能得到一些代码示例吗？我对此很新。 - dotnet-practitioner

@Servy - 是的，不过我假设那只是一个“例子”，并且有更多的枚举选项。 - Oded

@Oded：刚刚删除了 :) 哈哈... :D - Tigran

我喜欢它 - 它很优雅！然而，实现者最好确保枚举的所有值都已实现，或者首先使用Dictionary.Contains以防止任何调用者使用未覆盖的枚举值。 - dash

@dotnet-practitioner - 请看Eric Lippert的答案，其中使用了继承方法（如果您有多个不同的方法需要调用，则这是更好的方法）。 - Oded

Answer 3

在我看来，没有任何明显的理由需要更改你的代码。 这个函数很清晰，并且明确了if/else的定义。 函数声明使您可以按照您想要使用的方式使用它。

因此，我不会更改我看到的任何代码。

祝好运。

- Tigran

Answer 4

使用 switch block：

switch (logtype)
{
    case LogType.Debug:
        log.Debug(message);
        break;

    case LogType.Error:
        log.Error(message);
        break;

    //more cases here as needed...

    default:
        throw new InvalidArgumentException("logtype");
}

- phoog

Answer 5

总是可以使用switch语句：

switch (logtype)
{
    case LogType.Debug:
        log.Debug(message);
        break;
    case LogType.Error:
        log.Error(message);
        break;
    ....
}

- ChrisF

Answer 6

你可以使用 switch 或者创建一个以 LogType 为键，相应的 LogXXXX 方法作为值 - 一个 Action<string> 的字典，然后执行操作。

myDictionary[logType](message);

- MiMo