I was looking for a Ruby code quality tool the other day, and I came across the pelusa gem, which looks interesting. One of the things it checks for is the number of else statements used in a given Ruby file.
前几天我正在寻找一个Ruby代码质量工具,我遇到了pelusa gem,看起来很有趣。它检查的一件事是给定Ruby文件中使用的else语句的数量。
My question is, why are these bad? I understand that if/else
statements often add a great deal of complexity (and I get that the goal is to reduce code complexity) but how can a method that checks two cases be written without an else
?
我的问题是,为什么这些不好?我理解if / else语句经常会增加很多复杂性(我认为目标是降低代码复杂性)但是如何在没有else的情况下编写检查两种情况的方法?
To recap, I have two questions:
总结一下,我有两个问题:
1) Is there a reason other than reducing code complexity that else statements could be avoided?
1)除了降低代码复杂性之外还有其他原因可以避免其他语句吗?
2) Here's a sample method from the app I'm working on that uses an else
statement. How would you write this without one? The only option I could think of would be a ternary statement, but there's enough logic in here that I think a ternary statement would actually be more complex and harder to read.
2)这是我正在使用的应用程序使用else语句的示例方法。如果没有一个,你会怎么写?我能想到的唯一选择是三元语句,但这里有足够的逻辑,我认为三元语句实际上会更复杂,更难阅读。
def deliver_email_verification_instructions
if Rails.env.test? || Rails.env.development?
deliver_email_verification_instructions!
else
delay.deliver_email_verification_instructions!
end
end
If you wrote this with a ternary operator, it would be:
如果你用三元运算符写这个,那就是:
def deliver_email_verification_instructions
(Rails.env.test? || Rails.env.development?) ? deliver_email_verification_instructions! : delay.deliver_email_verification_instructions!
end
Is that right? If so, isn't that way harder to read? Doesn't an else
statement help break this up? Is there another, better, else
-less way to write this that I'm not thinking of?
是对的吗?如果是这样,是不是更难以阅读?不是其他声明有助于打破这种局面吗?是否有另一种更好的,更好的方式来写这个我没想到的?
I guess I'm looking for stylistic considerations here.
我想我在寻找风格方面的考虑因素。
2 个解决方案
#1
6
Let me begin by saying that there isn't really anything wrong with your code, and generally you should be aware that whatever a code quality tool tells you might be complete nonsense, because it lacks the context to evaluate what you are actually doing.
首先让我说你的代码没有任何问题,通常你应该知道,无论代码质量工具告诉你什么,都可能是完全无意义的,因为它缺乏评估你实际做什么的上下文。
But back to the code. If there was a class that had exactly one method where the snippet
但回到代码。如果有一个类只有一个方法所在的片段
if Rails.env.test? || Rails.env.development?
# Do stuff
else
# Do other stuff
end
occurred, that would be completely fine (there are always different approaches to a given thing, but you need not worry about that, even if programmers will hate you for not arguing with them about it :D).
发生了,这将是完全正常的(对于给定的事物总是有不同的方法,但你不必担心,即使程序员会讨厌你不与他们争论它:D)。
Now comes the tricky part. People are lazy as hell, and thusly code snippets like the one above are easy targets for copy/paste coding (this is why people will argue that one should avoid them in the first place, because if you expand a class later you are more likely to just copy and paste stuff than to actually refactor it).
现在是棘手的部分。人们很懒惰,因此像上面这样的代码片段是复制/粘贴编码的简单目标(这就是为什么人们会争辩说首先应该避免它们,因为如果你以后扩展课程你更有可能只是复制和粘贴东西,而不是实际重构它)。
Let's look at your code snippet as an example. I'm basically proposing the same thing as @Mik_Die, however his example is equally prone to be copy/pasted as yours. Therefore, would should be done (IMO) is this:
我们以您的代码片段为例进行说明。我基本上提出与@Mik_Die相同的东西,但是他的例子同样容易被复制/粘贴为你的。因此,应该做(IMO)是这样的:
class Foo
def initialize
@target = (Rails.env.test? || Rails.env.development?) ? self : delay
end
def deliver_email_verification_instructions
@target.deliver_email_verification_instructions!
end
end
This might not be applicable to your app as is, but I hope you get the idea, which is: Don't repeat yourself. Ever. Every time you repeat yourself, not only are you making your code less maintainable, but as a result also more prone to errors in the future, because one or even 99/100 occurrences of whatever you've copied and pasted might be changed, but the one remaining occurrence is what causes the @disasterOfEpicProportions
in the end :)
这可能不适用于你的应用程序,但我希望你明白这个想法:不要重复自己。永远。每次重复自己时,不仅会使代码的可维护性降低,而且将来也更容易出错,因为您复制和粘贴的任何内容都可能会发生一次甚至99/100次更改,但是剩下的一个是最终导致@disasterOfEpicProportions的原因:)
Another point that I've forgotten was brought up by @RayToal (thanks :), which is that if/else constructs are often used in combination with boolean input parameters, resulting in constructs such as this one (actual code from a project I have to maintain):
我忘记的另一点是@RayToal(感谢:),这是因为if / else构造经常与布尔输入参数结合使用,导致这样的结构(我有一个项目的实际代码)维持):
class String
def uc(only_first=false)
if only_first
capitalize
else
upcase
end
end
end
Let us ignore the obvious method naming and monkey patching issues here, and focus on the if/else construct, which is used to give the uc
method two different behaviors depending on the parameter only_first
. Such code is a violation of the Single Responsibility Principle, because your method is doing more than one thing, which is why you should've written two methods in the first place.
让我们在这里忽略明显的方法命名和猴子修补问题,并专注于if / else构造,它用于根据参数only_first为uc方法提供两种不同的行为。这样的代码违反了单一责任原则,因为你的方法不止一件事,这就是你应该首先编写两种方法的原因。
#2
2
def deliver_email_verification_instructions
subj = (Rails.env.test? || Rails.env.development?) ? self : delay
subj.deliver_email_verification_instructions!
end
#1
6
Let me begin by saying that there isn't really anything wrong with your code, and generally you should be aware that whatever a code quality tool tells you might be complete nonsense, because it lacks the context to evaluate what you are actually doing.
首先让我说你的代码没有任何问题,通常你应该知道,无论代码质量工具告诉你什么,都可能是完全无意义的,因为它缺乏评估你实际做什么的上下文。
But back to the code. If there was a class that had exactly one method where the snippet
但回到代码。如果有一个类只有一个方法所在的片段
if Rails.env.test? || Rails.env.development?
# Do stuff
else
# Do other stuff
end
occurred, that would be completely fine (there are always different approaches to a given thing, but you need not worry about that, even if programmers will hate you for not arguing with them about it :D).
发生了,这将是完全正常的(对于给定的事物总是有不同的方法,但你不必担心,即使程序员会讨厌你不与他们争论它:D)。
Now comes the tricky part. People are lazy as hell, and thusly code snippets like the one above are easy targets for copy/paste coding (this is why people will argue that one should avoid them in the first place, because if you expand a class later you are more likely to just copy and paste stuff than to actually refactor it).
现在是棘手的部分。人们很懒惰,因此像上面这样的代码片段是复制/粘贴编码的简单目标(这就是为什么人们会争辩说首先应该避免它们,因为如果你以后扩展课程你更有可能只是复制和粘贴东西,而不是实际重构它)。
Let's look at your code snippet as an example. I'm basically proposing the same thing as @Mik_Die, however his example is equally prone to be copy/pasted as yours. Therefore, would should be done (IMO) is this:
我们以您的代码片段为例进行说明。我基本上提出与@Mik_Die相同的东西,但是他的例子同样容易被复制/粘贴为你的。因此,应该做(IMO)是这样的:
class Foo
def initialize
@target = (Rails.env.test? || Rails.env.development?) ? self : delay
end
def deliver_email_verification_instructions
@target.deliver_email_verification_instructions!
end
end
This might not be applicable to your app as is, but I hope you get the idea, which is: Don't repeat yourself. Ever. Every time you repeat yourself, not only are you making your code less maintainable, but as a result also more prone to errors in the future, because one or even 99/100 occurrences of whatever you've copied and pasted might be changed, but the one remaining occurrence is what causes the @disasterOfEpicProportions
in the end :)
这可能不适用于你的应用程序,但我希望你明白这个想法:不要重复自己。永远。每次重复自己时,不仅会使代码的可维护性降低,而且将来也更容易出错,因为您复制和粘贴的任何内容都可能会发生一次甚至99/100次更改,但是剩下的一个是最终导致@disasterOfEpicProportions的原因:)
Another point that I've forgotten was brought up by @RayToal (thanks :), which is that if/else constructs are often used in combination with boolean input parameters, resulting in constructs such as this one (actual code from a project I have to maintain):
我忘记的另一点是@RayToal(感谢:),这是因为if / else构造经常与布尔输入参数结合使用,导致这样的结构(我有一个项目的实际代码)维持):
class String
def uc(only_first=false)
if only_first
capitalize
else
upcase
end
end
end
Let us ignore the obvious method naming and monkey patching issues here, and focus on the if/else construct, which is used to give the uc
method two different behaviors depending on the parameter only_first
. Such code is a violation of the Single Responsibility Principle, because your method is doing more than one thing, which is why you should've written two methods in the first place.
让我们在这里忽略明显的方法命名和猴子修补问题,并专注于if / else构造,它用于根据参数only_first为uc方法提供两种不同的行为。这样的代码违反了单一责任原则,因为你的方法不止一件事,这就是你应该首先编写两种方法的原因。
#2
2
def deliver_email_verification_instructions
subj = (Rails.env.test? || Rails.env.development?) ? self : delay
subj.deliver_email_verification_instructions!
end