应该何时尝试消除switch语句?

我在我正在处理的代码库中遇到了一个switch语句,我正在试图弄清楚如何用更好的东西替换它,因为switch语句被认为是代码味道 。 但是,通过阅读有关替换 switch 语句的 stackoverflow上的几篇文章,我似乎无法想到替换此特定switch语句的有效方法。

它让我想知道这个特定的switch语句是否正常,以及是否有特殊情况认为switch语句是合适的。

在我的情况下,我正在努力的代码(自然略微混淆)是这样的:

private MyType DoSomething(IDataRecord reader) { var p = new MyType { Id = (int)reader[idIndex], Name = (string)reader[nameIndex] } switch ((string) reader[discountTypeIndex]) { case "A": p.DiscountType = DiscountType.Discountable; break; case "B": p.DiscountType = DiscountType.Loss; break; case "O": p.DiscountType = DiscountType.Other; break; } return p; } 

谁能建议一种消除这种转换的方法? 或者这是一个适当的开关? 如果是,那么switch语句还有其他适当的用途吗? 我真的很想知道它们的适用位置,所以我不会浪费太多时间来消除我遇到的每一个开关语句,因为它们在某些情况下被认为是一种气味。

更新:根据迈克尔的建议,我做了一些搜索这个逻辑的重复,发现有人在另一个类中创建了逻辑,有效地使整个switch语句变得多余。 所以在这个特定代码的上下文中,switch语句是不必要的。 但是,我的问题更多的是关于代码中switch语句的适当性以及我们是否应该总是尝试在找到它们时替换它们,所以在这种情况下我倾向于接受这个switch语句是合适的答案。

这适用于开关语句,因为它使选择可读,并且易于添加或减去一个。

看到这个链接。

切换语句(特别是长语句)被认为是错误的,不是因为它们是switch语句,而是因为它们的存在表明需要重构。

switch语句的问题是它们在代码中创建了一个分叉(就像if语句一样)。 每个分支必须单独测试,每个分支内的每个分支……嗯,你明白了。

也就是说,以下文章有一些关于使用switch语句的好习惯:

http://elegantcode.com/2009/01/10/refactoring-a-switch-statement/

对于您的代码,上面链接中的文章表明,如果您正在执行从一个枚举到另一个枚举的这种类型的转换,您应该将您的开关放在它自己的方法中,并使用return语句而不是break语句。 我以前做过这个,代码看起来更干净:

 private DiscountType GetDiscountType(string discount) { switch (discount) { case "A": return DiscountType.Discountable; case "B": return DiscountType.Loss; case "O": return DiscountType.Other; } } 

我认为为了改变代码而改变代码并不是最好的利用时间。 更改代码使其[更具可读性,更快速,更高效等]是有道理的。 不要仅仅因为有人说你做的事情“改变”而改变它。

-Rick

这个switch语句很好。 你们有没有其他的错误要注意? 大声笑

但是,有一点我注意到了……你不应该在IReader []对象索引器上使用索引序数….如果列顺序改变了怎么办? 尝试使用字段名称,例如reader [“id”]和reader [“name”]

在我看来,它不是切换语句的气味,而是它们内部的内容。 对我来说,这个switch语句没问题,直到它开始添加几个案例。 那么可能值得创建一个查找表:

 private static Dictionary DiscountTypeLookup = new Dictionary(StringComparer.Ordinal) { {"A", DiscountType.Discountable}, {"B", DiscountType.Loss}, {"O", DiscountType.Other}, }; 

根据您的观点,这可能或多或少可读。

如果你的案件的内容超过一两行,那么事情开始变得臭。

Robert Harvey和Talljoe提供了很好的答案 – 你在这里有一个从字符代码到枚举值的映射。 这最好表示为映射,其中映射的详细信息在一个地方提供,或者在地图中(如Talljoe建议的)或在使用switch语句的函数中提供(如Robert Harvey所建议的)。

在这种情况下,这两种技术都可能很好,但我想提请你注意一个在这里或其他类似情况下可能有用的设计原理。 开放/封闭校长

如果映射可能会随着时间的推移而改变 ,或者可能是扩展的运行时(例如,通过插件系统或通过从数据库中读取映射的部分),那么使用注册表模式将帮助您坚持打开/关闭实际上,允许扩展映射而不影响使用映射的任何代码(正如他们所说 – 打开扩展,关闭以进行修改)。

我认为这是关于注册表模式的一篇很好的文章 – 看看注册表如何保存从某个键到某个值的映射? 以这种方式,它类似于表示为switch语句的映射。 当然,在您的情况下,您将不会注册所有实现公共接口的对象,但您应该获得要点:

因此,回答原始问题 – case语句是不好的forms,因为我希望在应用程序的多个位置需要从字符代码到枚举值的映射,因此应该将其考虑在内。 我引用的两个答案为您提供了很好的建议 – 根据您的喜好选择。 但是,如果映射可能会随着时间的推移而发生变化,请将注册表模式视为使代码免受此类更改影响的方法。

我不会用ifif不如switch明显。 switch告诉我你在整个过程中都在比较同样的事情。

只是为了吓唬人,这不如你的代码清晰:

 if (string) reader[discountTypeIndex]) == "A") p.DiscountType = DiscountType.Discountable; else if (string) reader[discountTypeIndex]) == "B") p.DiscountType = DiscountType.Loss; else if (string) reader[discountTypeIndex]) == "O") p.DiscountType = DiscountType.Other; 

这个switch可能没问题,你可能想看看@Talljoe的建议。

您的代码中是否包含折扣类型的开关? 添加新的折扣类型是否需要您修改几个这样的开关? 如果是这样,你应该考虑将开关考虑在内。 如果没有,在这里使用开关应该是安全的。

如果整个程序中存在大量折扣特定行为,您可能希望重构这样:

 p.Discount = DiscountFactory.Create(reader[discountTypeIndex]); 

然后折扣对象包含与计算折扣相关的所有属性和方法。

您怀疑此switch语句是正确的:任何依赖于某种类型的switch语句都可能表示缺少多态(或缺少子类)。

然而,TallJoe的字典是一种很好的方法。

请注意, 如果您的枚举和数据库值是整数而不是字符串,或者您的数据库值与枚举名称相同,那么reflection将起作用,例如给定

 public enum DiscountType : int { Unknown = 0, Discountable = 1, Loss = 2, Other = 3 } 

然后

 p.DiscountType = Enum.Parse(typeof(DiscountType), (string)reader[discountTypeIndex])); 

就够了

是的,这看起来像是switch语句的正确用法。

但是,我还有另一个问题要问你。

为什么没有包含默认标签? 在默认标签中抛出exception将确保在添加新的discountTypeIndex时忘记修改代码时程序将无法正常运行。

此外,如果要将字符串值映射到枚举,可以使用属性和reflection。

就像是:

 public enum DiscountType { None, [Description("A")] Discountable, [Description("B")] Loss, [Description("O")] Other } public GetDiscountType(string discountTypeIndex) { foreach(DiscountType type in Enum.GetValues(typeof(DiscountType)) { //Implementing GetDescription should be easy. Search on Google. if(string.compare(discountTypeIndex, GetDescription(type))==0) return type; } throw new ArgumentException("DiscountTypeIndex " + discountTypeIndex + " is not valid."); } 

我认为这取决于你是否正在创建MType添加许多不同的地方或仅在这个地方。 如果你在许多地方创建MType总是不得不切换dicsount类型有一些其他检查,那么这可能是代码气味。

我会尝试在你的程序中的一个单独的位置创建MTypes可能在MType本身的构造函数中或在某种工厂方法中,但是程序的随机部分赋值可能导致有人不知道值应该如何做错了。

所以开关很好但是开关需要在Type的创建部分内移动更多

我并不是绝对反对switch语句,但是在你出现的情况下,我至少已经消除了分配DiscountType的重复; 我可能会改为编写一个返回给定字符串的DiscountType的函数。 该函数可以简单地为每个案例提供返回语句,从而消除了rest的需要。 我发现开关盒之间需要断裂非常危险。

 private MyType DoSomething(IDataRecord reader) { var p = new MyType { Id = (int)reader[idIndex], Name = (string)reader[nameIndex] } p.DiscountType = FindDiscountType(reader[discountTypeIndex]); return p; } private DiscountType FindDiscountType (string key) { switch ((string) reader[discountTypeIndex]) { case "A": return DiscountType.Discountable; case "B": return DiscountType.Loss; case "O": return DiscountType.Other; } // handle the default case as appropriate } 

很快,我注意到FindDiscountType()确实属于DiscountType类并移动了该函数。

当您设计一种语言并最终有机会删除整个语言中最丑陋,最直观的容易出错的语法。

这是当您尝试删除switch语句时。

为了清楚起见,我的意思是语法。 这是从C / C ++中获取的东西,它应该被改变以符合C#中更现代的语法。 我完全同意提供开关的概念,因此编译器可以优化跳转。