考虑一个无参数(无需编辑)函数,该函数执行一行代码,并且在程序中仅被调用一次(尽管将来再次使用它并非不可能)。

它可以执行查询,检查某些值,执行涉及正则表达式的操作...晦涩或“ hacky”的任何操作。

其背后的原理是避免难以理解的评估:

 if (getCondition()) {
    // do stuff
}
 


其中getCondition()是单行函数。

我的问题很简单:这是一个好习惯吗?对我来说似乎不错,但我不知道长期...

评论

除非您的代码片段来自带有隐式接收器的OOP语言(例如this),否则这肯定是一个坏习惯,因为getCondition()最有可能依赖于全局状态...

他可能是想暗示getCondition()可以有参数?

@Ingo-有些东西确实具有全局状态。当前时间,主机名,端口号等均为有效的“全局”。设计错误使“全局”脱离了本来就不是全局的东西。

为什么不直接内联getCondition?如果它像您所说的那样小且不经常使用,那么给它起个名字并不能解决任何问题。

davidk01:代码可读性。

#1 楼

取决于那一行。如果该行本身可读且简洁,则可能不需要该功能。简单的例子:



 void printNewLine() {
  System.out.println();
}
 


OTOH,如果函数给包含例如一个复杂的,难以理解的表达,对我来说是完全合理的。人为的示例(为了便于阅读,分成多行):

 boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}
 


评论


+1。这里的魔术词是“自我记录代码”。

–Konamiman
2011年9月12日15:17

鲍勃叔叔称之为“封装条件”的一个很好的例子。

–安东尼·佩格拉姆
2011年9月12日在18:07

@Aditya:在这种情况下,没有人说taxPayer是全球性的。此类可能是TaxReturn,而taxPayer是一个属性。

–亚当·罗宾逊(Adam Robinson)
2011年9月12日在22:14

此外,它还允许您以某种方式记录功能,例如javadoc并公开可见。

–user1249
2011年9月13日在7:04

@dallin,鲍勃·马丁(Bob Martin)的“干净代码”(Clean Code)显示了许多半现实的代码示例。如果单个类中的函数太多,那么您的类太大了吗?

–彼得·托克(PéterTörök)
2013年9月6日16:09

#2 楼

是的,可以用来满足最佳实践。例如,最好是使用一个命名明确的函数来完成某些工作,即使它只有一行,也要比将一行代码包含在一个更大的函数中,并且需要用一行注释来说明其功能。同样,相邻的代码行应以相同的抽象级别执行任务。一个反例可能是类似

startIgnition();
petrolFlag |= 0x006A;
engageChoke();


,在这种情况下,最好将中间行移动到一个有意义的函数中。

评论


那个0x006A很可爱:D这是众所周知的带有添加剂a,b和c的碳化燃料常数。

–编码器
2011-09-12 15:33

+1我全都是自我记录代码:)顺便说一句,我想我同意通过块保持恒定的抽象级别,但是我无法解释原因。您介意扩大吗?

–vemv
2011-09-12 17:18

如果您只是说petrolFlag | = 0x006A;如果没有任何决策,最好只说petrolFlag | = A_B_C;没有其他功能。据推测,只有在petrolFlag满足特定条件时才应调用engageChoke(),并且应明确指出“我在这里需要一个函数”。只是一个小问题,这个答案基本上在其他地方都可以找到:)

– Tim Post
2011-09-12 18:56



+1指出方法中的代码应处于同一抽象级别。 @vemv,这是一个好习惯,因为它避免了在阅读代码的同时在不同抽象级别之间跳来跳去,从而使代码更易于理解。将抽象级别的开关绑定到方法调用/返回(即结构“跳转”)是使代码更加流畅和简洁的好方法。

–彼得·托克(PéterTörök)
2011-09-13 6:55



不,这是完全虚构的价值。但是,您对此感到奇怪的事实只是强调了这一点:如果代码说了mixLeadedGasoline(),您将不必这样做!

–基连·福斯
2011年9月13日在8:22

#3 楼

我认为,在很多情况下,这样的函数是不错的样式,但是在不需要在其他地方使用此条件的情况下,可以考虑使用局部布尔变量作为替代,例如: pre class =“ lang-c prettyprint-override”> bool someConditionSatisfied = [complex expression];

这将为代码阅读器提供提示,并避免引入新功能。

评论


如果条件函数名称难以理解或可能引起误解(例如IsEngineReadyUnlessItIsOffOrBusyOrOutOfService),则布尔值特别有用。

– dbkk
2011年9月12日20:01在

我遇到了这个建议,这是一个坏主意:布尔值和条件会分散对功能核心业务的关注。同样,更喜欢局部变量的样式使重构更加困难。

–狼
2015年1月9日在9:22



@Wolf OTOH,我更喜欢此方法而不是函数调用,以减少代码的“抽象深度”。 IMO,跳转到函数是比显式和直接的几行代码更大的上下文切换,尤其是如果它仅返回布尔值时。

– Kache
16-10-19在4:44

@Kache我认为这取决于您是否编写面向对象的代码,在OOP中,成员函数的使用使设计更加灵活。这真的取决于环境...

–狼
16-10-19在6:03

#4 楼

除了Peter的回答外,如果将来可能需要更新该条件,则可以通过将其封装为建议的方式来封装,建议您只有一个编辑点。

按照彼得的例子,如果这



 boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isFemale() 
        && (taxPayer.getNumberOfChildren() > 2 
        || (taxPayer.getAge() > 50 && taxPayer.getEmployer().isNonProfit()));
}
 


成为


 boolean isTaxPayerEligibleForTaxRefund() {
  return taxPayer.isMutant() 
        && (taxPayer.getNumberOfThumbs() > 2 
        || (taxPayer.getAge() > 123 && taxPayer.getEmployer().isXMan()));
}
 


您进行一次编辑并更新普遍地在可维护性方面,这是一个加号。

在性能方面,大多数优化的编译器都会删除函数调用并内联小代码块。优化这样的事情实际上可以缩小块的大小(通过删除函数调用,return等所需的指令),因此即使在可能会阻止内联的条件下,这样做通常也是安全的。

评论


我已经意识到保持单个编辑点的好处-这就是为什么问题说“ ...被调用一次”的原因。尽管如此,很高兴了解这些编译器优化,我一直认为它们确实遵循了此类说明。

–vemv
2011-09-12 17:16

拆分测试简单条件的方法往往意味着可以通过更改方法来更改条件。如果确实如此,则这种暗示很有用,但如果事实并非如此,则可能很危险。例如,假设代码需要将事物划分为轻量对象,绿色重对象和非绿色重对象。对象具有快速的“ seemsGreen”属性,该属性对于重的对象是可靠的,但对于轻量的对象可能会返回误报。它们还具有“ measureSpectralResponse”功能,该功能虽然很慢,但将对所有对象可靠地工作。

–超级猫
2014年9月2日19:37



如果代码从不关心轻量级对象是否为绿色,则似乎绿色与轻量级对象不可靠的事实将是无关紧要的。但是,如果“轻量级”的定义发生了变化,以致某些未返回为true的非绿色对象没有被报告为“轻量级”,那么对“轻量级”的定义的这种更改可能会破坏测试对象为“绿色”。在某些情况下,与单独的方法相比,在代码中一起测试绿色和重量可以使测试之间的关系更清晰。

–超级猫
2014年9月2日19:41

#5 楼

除了可读性(或作为补充)之外,这还允许在适当的抽象级别上编写函数。

评论


恐怕我不明白你的意思...

–vemv
2011-09-12 20:28

@vemv:我认为他的意思是“适当的抽象级别”,您不会最终得到混合了不同抽象级别的代码(即Kilian Foth已经说过的话)。当周围的代码正在考虑当前情况的更高级视图(例如运行引擎)时,您不希望代码处理看似无关紧要的细节(例如燃料中所有键的形成)。前者会使后者混乱,因为您将不得不随时考虑所有抽象级别,从而使您的代码难以阅读和维护。

– Egon
2011年9月12日在22:02

#6 楼

这取决于。有时,最好将表达式封装在函数/方法中,即使它只是一行。如果阅读起来很复杂,或者您需要在多个地方阅读它,那么我认为这是一个好习惯。从长远来看,由于您已经引入了单点更改并具有更好的可读性,因此维护起来更容易。

但是,有时它只是您不需要的东西。如果该表达式仍然很容易阅读和/或仅出现在一个地方,则不要将其包装。

#7 楼

我认为,如果您只有少数几个,那就可以了,但是当您的代码中有很多它们时,就会出现问题。而当编译器运行或插入器(取决于您使用的语言)时,它将进入内存中的该函数。因此,假设您有3个,我认为计算机不会注意到,但是如果您开始拥有100个这些小东西,则系统必须在内存中注册仅被调用一次且不会销毁的函数。

评论


根据Stephen的回答,这可能并不总是会发生(尽管盲目地相信编译器的魔力还是不好的)

–vemv
2011年9月13日下午5:59

是的,应根据许多事实将其清除。如果它是一种插补的语言,则系统每次都会落入单行功能,除非您为高速缓存安装了某些东西,但仍然无法解决问题。现在,当涉及到编译器时,仅在虚弱的一天和刨刀的位置发生问题才重要,如果编译器将成为您的朋友并清除您的小麻烦,或者它是否会思考您是否真的需要它。我想提醒一下,如果您知道循环的确切时间,那么每次将其复制并粘贴多次后,总会更好地运行循环。

– WojonsTech
2011年9月13日下午6:07

+1对准行星:)但您的最后一句话听起来对我来说完全是疯狂的,您真的这样做吗?

–vemv
2011年9月13日在7:41

这实际上取决于大多数情况下,不,除非我获得正确的付款金额以检查它是否确实加速了诸如此类的速度,否则我不会这样做。但是在较旧的编译器中,最好先将其复制并粘贴,然后让编译器将其找出9/10。

– WojonsTech
2011年9月14日下午0:58

#8 楼

我刚刚在重构的应用程序中做了此确切的事情,以明确代码的实际含义(不带注释):

 protected void PaymentButton_Click(object sender, EventArgs e)
    Func<bool> HaveError = () => lblCreditCardError.Text == string.Empty && lblDisclaimer.Text == string.Empty;

    CheckInputs();

    if(HaveError())
        return;

    ...
}
 


评论


只是好奇,那是什么语言?

–vemv
2011年9月12日20:27在

@vemv:在我看来像C#。

–斯科特·米切尔(Scott Mitchell)
2011年9月12日在20:37

与注释相比,我也更喜欢使用额外的标识符。但这是否真的不同于引入局部变量以使if短?这种本地(lambda)方法使PaymentButton_Click函数(整体)更难以阅读。您的示例中的lblCreditCardError似乎是成员,因此HaveError也是对对象有效的(私有)谓词。我倾向于对此表示反对,但是我不是C#程序员,所以我拒绝了。

–狼
2015年1月9日在9:35



@Wolf嘿,是的。我很早以前就写了这篇文章:)如今,我做事的方式绝对不同。实际上,查看标签内容以查看是否有错误使我感到畏缩...为什么我不只是从CheckInputs()返回布尔值???

– joshperry
2015年1月9日在17:38

#9 楼

将这一行移到一个命名良好的方法中可使您的代码更易于阅读。许多其他人已经提到了这一点(“自我记录代码”)。将其移至方法中的另一个优点是,它使单元测试更加容易。如果将其隔离到自己的方法中并进行了单元测试,则可以确定是否/当发现错误时,该方法中不会存在该错误。

#10 楼

已经有很多不错的答案,但是有一个特殊的情况值得一提。

如果您的单行语句需要注释,并且您能够清楚地识别(意思是:名称)目的,请考虑提取一个函数,同时将注释增强到API文档中。通过这种方式,您可以使函数调用更轻松,更容易理解。

有趣的是,如果当前无事可做,可以做同样的事情,但是有一条注释提醒需要扩展(在不久的将来)1。 ),因此,



 def sophisticatedHello():
    # todo set up
    say("hello")
    # todo tear down
 


也可以变成了这个

 def sophisticatedHello():
    setUp()
    say("hello")
    tearDown()
 



1)您应该对此非常确定(请参阅YAGNI原理)

#11 楼

如果语言支持,我通常使用带标签的匿名函数来完成此操作。



 someCondition = lambda p: True if [complicated expression involving p] else False
#I explicitly write the function with a ternary to make it clear this is a a predicate
if (someCondition(p)):
    #do stuff...
 


恕我直言,这是一个很好的折衷方案,因为它为您带来了可读性,即避免了复杂的表达式使if条件混乱,同时又避免了带有小的一次性标签的全局/软件包命名空间的混乱。它的另一个好处是,函数“ definition”位于正确的位置,因此可以轻松修改和读取定义。

它不必只是谓词函数。我也喜欢将重复的样板包含在这样的小函数中(它对于生成Pythonic列表特别有用,而且不会弄乱括号的语法)。例如,以下在python

 #goal - I have a list of PIL Image objects and I want them all as grayscale (uint8) numpy arrays
im_2_arr = lambda im: array(im.convert('L')) 
arr_list = [im_2_arr(image) for image in image_list]
 

中使用PIL时,以下简化的示例

评论


为什么要使用“ lambda p:如果[涉及p的复杂表达式]否,则为True”,而不是“ lambda p:[涉及p的复杂表达式]”? 8)

–汉斯·彼得·斯托尔
2011年9月13日下午6:42

@hstoerr其在该行下方的注释中。我想明确表明我们someCondition是一个谓词。尽管这完全没有必要,但我写了很多科学脚本,但编写的代码不那么多,我个人认为具有额外的简洁性而不是让同事困惑是更合适的,因为他们不知道= False或其他并非总是“直观”的类似pythonic等价物。实际上,它实际上是标记someCondition是谓词的一种方式。

–疯狂
2011-09-13 6:56



只是为了清除我的明显错误[]!= False,但当将其转换为布尔值时,[]是False

–疯狂
2011年9月13日在7:03

@crasic:当我不希望我的读者知道[]的结果为False时,我宁愿执行len([生成列表的复杂表达式])== 0,而不是如果[blah]否则为True则仍然需要False读者知道[]计算为False。

– Lie Ryan
2011年9月13日在7:33