最近的错误修复要求我查看其他团队成员编写的代码(在C#中):

return (decimal)CostIn > 0 && CostOut > 0 ? (((decimal)CostOut - (decimal)CostIn) / (decimal)CostOut) * 100 : 0;


现在,允许有充分的理由所有这些演员,这仍然很难遵循。计算中有一个小错误,我不得不解开它以解决问题。当然,这里还有价值:我们所有人都看到了不必要的复杂条件逻辑链,这些逻辑可以与一些位置合理的运算符一起整理。但是,在跟着一连串的操作员挤成一个语句时,他显然比我更熟练。但是,是否有任何关于识别代码简洁性不再有用并成为理解障碍的观点的书面著作或研究?

强制转换的原因是实体框架。数据库需要将它们存储为可空类型。十进制?与C#中的Decimal不等效,需要进行强制转换。

评论

当简洁胜过可读性时。

看您的特定示例:强制转换是(1)开发人员比编译器了解更多的地方,并且需要告诉编译器无法推论的事实,或者(2)在“错误的地方”存储了一些数据。 ”,以表示我们需要对其执行的各种操作。两者都是可以重构的有力指标。最好的解决方案是找到一种无需强制转换即可编写代码的方法。

尤其奇怪的是,必须将CostIn强制转换为十进制以将其与零进行比较,而不是CostOut;这是为什么?到底CostIn的类型是什么,它只能通过将其转换为十进制而与零进行比较?为什么CostOut与CostIn的类型不同?

而且,这里的逻辑实际上可能是错误的。假设CostOut等于Double.Epsilon,因此大于零。但是在这种情况下,(decimal)CostOut为零,并且我们有一个除以零的误差。第一步应该是使代码正确,但我认为事实并非如此。使其正确,制作测试用例,然后使其优雅。优雅的代码和简短的代码有很多共同点,但是有时简洁不是优雅的灵魂。

简洁永远是一种美德。但是我们的目标功能将简洁与其他优点结合在一起。如果在不损害其他美德的情况下可以简短一点,那总是应该的。

#1 楼

要回答有关现存研究的问题


但是,是否有任何书面或研究发现认识到精简代码的努力不再有用并成为理解的障碍?


是的,这方面已经开展了工作。定量的基础(而不是像其他答案那样仅基于机智和直觉进行比较)。已查看的一种潜在度量标准是

循环复杂度÷代码源代码行(SLOC)

在您的代码示例中,该比率非常高,因为所有


SATC发现最有效的评估是大小和[Cyclomatic]复杂度的组合。具有高复杂度和大尺寸的模块往往具有最低的可靠性。小尺寸和高复杂度的模块也存在可靠性风险,因为它们往往是非常简洁的代码,难以更改或修改。


链接

如果您感兴趣,这里有一些参考资料:

McCabe,T.和A. Watson(1994),软件复杂性(CrossTalk:国防软件工程杂志)。 > Watson,AH和McCabe,TJ(1996)。结构化测试:使用环复杂度度量的测试方法(NIST特殊出版物500-235)。从McCabe Software网站检索:2011年5月14日:http://www.mccabe.com/pdf/mccabe-nist235r.pdf

Rosenberg,L.,Hammer,T.,Shaw,J (1998)。软件度量标准和可靠性(IEEE国际软件可靠性工程研讨会论文集)。从宾夕法尼亚州立大学网站检索:2011年5月14日:http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.104.4041&rep=rep1&type=pdf

我的意见和解决方案

就我个人而言,我从来没有重视简洁性,只有可读性。有时简洁有助于提高可读性,有时却不能。更重要的是,您是在编写真正的明显代码(ROC)而不是仅写代码(WOC)。要写出来:消除了对decimal的强制转换。

评论


我非常喜欢小于零的情况的保护条款。可能值得一提:成本怎么可能小于零,那是什么特例?

–user949300
17年5月5日在17:05

+1。我可能会添加一些类似的东西,如果((costIn <0)||(costOut <0))抛出新的Exception(“ costs不能为负”);

–布朗博士
17年1月5日,18:01



@DocBrown:这是一个很好的建议,但是我会考虑是否可以通过测试来执行特殊的代码路径。如果是,则编写一个测试该代码路径的测试用例。如果否,则将整个内容更改为断言。

–埃里克·利珀特
17年1月5日在18:34

尽管这些都是好点,但我相信这个问题的范围是代码风格,而不是逻辑。从黑匣子的角度来看,我的代码剪裁是在功能等效性上的努力。引发异常与返回0并不相同,因此解决方案在功能上将不等效。

–吴宗宪
17年1月5日在23:23



“就我个人而言,我从来没有重视简洁性,只重视可读性。有时简洁有助于可读性,有时则不。优点。为此+1。我也喜欢您的代码解决方案。

–通配符
17年1月6日在3:48

#2 楼

精简可以减少周围重要事物的混乱,但是当它变得简洁时,将太多相关数据打包得过于密集而无法轻易跟随,那么相关数据本身就会变得混乱,您就会遇到麻烦。

在这种情况下,对decimal的强制转换会不断重复。总体上来说,将其重写为以下内容可能会更好:

var decIn = (decimal)CostIn;
var decOut = (decimal)CostOut;
return decIn > 0 && CostOut > 0 ? (decOut - decIn ) / decOut * 100 : 0;
//                  ^ as in the question


突然,包含逻辑的线短了很多,并且可以装在一条水平线上,因此您可以无需滚动即可查看所有内容,其含义更加明显。

评论


我可能会走得更远并重构((decOut-decIn)/ decOut)* 100到另一个变量。

– FrustratedWithFormsDesigner
17年5月5日在16:39

汇编程序更加清晰:每行仅执行一次操作。 h!

–user251748
17年1月5日在18:21

@FrustratedWithFormsDesigner我甚至可以更进一步,将条件检查放在括号中。

–克里斯·西里菲斯(Chris Cirefice)
17年1月5日在18:23

@FrustratedWithFormsDesigner:将其提取到条件条件之前将绕过零除检查(CostOut> 0),因此您必须将条件扩展为if语句。这并不是说有什么问题,但它确实增加了更多的冗长性,而不仅仅是引入本地语言。

– wchargin
17年1月6日在5:22

坦白地说,如果有人因为难以识别简单的基本费率计算而难以阅读,那么摆脱强制转换就好像您对IMO足够了,这是他的问题,而不是我的问题。

–沃尔夫特
17年1月6日,13:21



#3 楼

尽管我无法对此主题进行任何特定的研究,但我建议您代码中的所有强制类型转换都违反了“不要自己重复”的原则。您的代码似乎正在尝试将costIncostOut转换为Decimal类型,然后对此类转换的结果执行完整性检查,如果检查通过则对这些转换后的值执行其他操作。实际上,您的代码对未转换的值执行完整性检查之一,从而增加了costOut可能保留大于零但小于Decimal可以表示的最小非零大小的一半的可能性。如果代码定义了Decimal类型的变量来保存转换后的值,然后对这些值进行操作,则代码将更加清晰。 DecimalcostIn的值要比costOutcostIn的实际值之比大一点,除非代码还将将十进制表示形式用于其他目的。如果代码将要进一步使用这些表示形式,那将是创建变量以保存这些表示形式的进一步论据,而不是在整个代码中连续进行强制转换。

评论


(十进制)强制类型转换可能是为了满足某些业务规则要求。与会计师打交道时,有时您不得不跳过愚蠢的篮球。我以为CFO会发现“使用税收舍入更正-0.01美元”这一行在考虑到所要求的功能后不可避免,将导致心脏病发作。 (提供:税后价格。我必须计算税前价格。没有答案的几率等于税率。)

–Loren Pechtel
17年1月5日在23:23

@LorenPechtel:鉴于小数转换的精度取决于所讨论的值的大小,因此我很难想象业务规则会要求转换的实际行为方式。

–超级猫
17年1月6日,0:09



好一点-我忘记了十进制类型的细节,因为我从来没有机会想要这样的东西。我现在认为这是对商业规则的狂热崇拜,特别是金钱一定不能浮于水面。

–Loren Pechtel
17年1月6日,0:15

@LorenPechtel:澄清我的最后一点:定点类型可以保证x + y-y会溢出或产生y。小数类型不是。值1.0d / 3.0的十进制小数位数比使用较大数字时可以保留的位数多。因此,相加然后减去相同的较大数字将导致精度损失。定点类型可能会因分数乘法或除法而失去精度,但不会因加,减,乘或除以余数而失精度(例如1.00 / 7为0.14余数0.2; 1.00 div 0.15为6余数0.10)。

–超级猫
17年1月6日15:21



@绿巨人:是的,当然。我正在争论是否使用x + y-y产生x,x + y-x产生y或x + y-x-y,最后对前两个进行了密切处理。重要的是,定点类型可以保证许多操作序列永远不会导致任何程度的不可检测的舍入误差。如果代码以不同的方式将事物加在一起以验证总数是否匹配(例如,将行小计的总和与列小计的总和进行比较),则使结果精确地相等要比使它们“接近”好得多。

–超级猫
17年1月9日在15:28

#4 楼

当我们忘记了达到目的的手段而不是本身的美德时,简洁就不再是一种美德。我们喜欢简洁,因为它与简单性相关;我们喜欢简单性,因为更简单的代码更易于理解,易于修改并且包含更少的错误。最后,我们希望代码实现以下目标:


以最少的工作量满足业务需求
避免错误
让我们在以下方面进行更改继续实现1和2的未来

这些是目标。任何设计原则或方法(无论是KISS,YAGNI,TDD,SOLID,证明,类型系统,动态元编程等)在它们有助于我们实现这些目标的范围内都是有益的。

所讨论的台词似乎错过了最终目标。虽然很短,但并不简单。通过多次重复相同的铸造操作,实际上包含不必要的冗余。重复代码会增加复杂性并增加错误的可能性。将转换与实际计算混合在一起也使代码难以遵循。

该行涉及三个方面:防护(特殊套管0),类型转换和计算。孤立地考虑每个问题都非常简单,但是由于所有问题都被混合在同一个表达式中,因此很难遵循。二手威乐Q4312079Q是。可能有充分的理由,但是意图并不明确(至少并非没有上下文),这意味着维护人员会警惕更改此代码,因为可能存在一些隐藏的假设。这是对可维护性的厌恶。

由于CostOut是在与0比较之前强制转换的,因此我假设它是一个浮点值。 (如果它是一个int,则没有理由强制转换)。但是,如果CostIn是浮点数,则代码可能会隐藏一个晦涩的被零除的bug,因为浮点值可能很小但非零,但当转换为十进制时为零(至少我相信这是可能的。 )

所以问题不是简洁或缺乏,问题是重复的逻辑和关注点的混淆导致难以维护的代码。

引入变量以保留强制转换的值可能会增加按代数计算的代码的大小,但会降低复杂性,分离问题并提高清晰度,这使我们更接近于代码目标更易于理解和维护。

评论


重要的一点是:强制转换CostIn一次而不是两次使它变得不可读,因为读者不知道这是否是一个带有明显修复的细微错误,还是是否有意这样做。显然,如果我不能确定说一句作者是什么意思,那么它就不可读。应该有两个强制转换,或者要有注释来解释为什么第一次使用CostIn不需要或不应该强制转换。

– gnasher729
19年6月15日在23:44

#5 楼

我查看该代码,然后问:“成本如何为0(或更少)?”。这表示什么特殊情况?代码应为

bool BothCostsAreValidProducts = (CostIn > 0) && (CostOut > 0);
if (!BothCostsAreValidProducts)
  return NO_PROFIT;
else {
   // that calculation here...
}


我想在这里命名:适当更改BothCostsAreValidProductsNO_PROFIT

评论


当然,成本可以为零(以圣诞节为例)。在负利率时代,负成本也不应该太令人惊讶,至少代码应该准备好应对(并通过抛出错误来解决)

–哈根·冯·埃森(Hagen von Eitzen)
17年1月6日在16:06

您走对了。

–danny117
17年1月6日在17:05

真傻if(CostIn <= 0 || CostOut <= 0)完全可以。

–Miles Rout
17年1月10日在20:37

可读性差得多。变量名称太可怕了(BothCostsAreValid会更好。这里没有关于产品的信息)。但是即使那样也不会增加任何可读性,因为仅检查CostIn,CostOut就可以了。如果要测试的表达式的含义不明显,则会引入一个有意义的名称作为额外的变量。

– gnasher729
19年6月15日在23:41

#6 楼

简洁根本不是美德。可读性是美德。

简洁可以成为实现美德的工具,或者像您的示例一样,可以成为实现完全相反的工具。这样,它几乎没有任何价值。代码应该“尽可能短”的规则也可以用“尽可能淫秽”代替-它们同样没有意义,并且如果没有更大的用途,则可能造成破坏。

此外,您发布的代码甚至都不遵循简短规则。如果用M后缀声明常量,则可以避免大多数可怕的(decimal)强制类型转换,因为编译器会将剩余的int提升为decimal。我相信您所描述的人只是以简洁为借口。很可能不是故意的,但仍然是。

#7 楼

在多年的经验中,我开始相信最终的简洁就是时间-时间主导着其他一切。这包括性能时间-程序完成工作需要多长时间-以及维护时间-添加功能或修复错误需要多长时间。 (如何平衡两者取决于执行和改进代码的频率-请记住,过早的优化仍然是万恶之源。)简短的代码是为了提高两者的简洁性。较短的代码通常运行速度更快,并且通常更易于理解和维护。如果它也不做,那就是净负数。

在这里显示的情况下,我认为文本的简洁性被误认为行数的简洁性,以牺牲可读性为代价,这可能增加维护时间。 (取决于转换的完成方式,它可能还需要更长的时间才能执行,但是除非上一行运行了数百万次,否则可能不必担心。)在这种情况下,重复的十进制强制转换会降低可读性,因为这样做更难看看最重要的计算是什么。我会这样写:

decimal dIn = (decimal)CostIn;
decimal dOut = (decimal)CostOut;
return dIn > 0 && CostOut > 0 ? ((dOut - dIn) / dOut) * 100 : 0;


(编辑:这是与其他答案相同的代码,所以就可以了。) >我是三元运算符? :的粉丝,所以我将其保留。

评论


三元数很难阅读,特别是如果返回值中除了单个值或变量之外还存在其他表达式时。

– Almo
17年1月5日在18:40

我想知道这是否是导致否定投票的原因。除非我写的内容与梅森·惠勒(Mason Wheeler)大致一致,目前票数为10票。他也离开了三元。我不知道为什么这么多人有问题? :-我认为上面的示例足够紧凑,尤其是。与if-then-else相比。

– Paul Brinkley
17年5月5日在19:32

真的不确定。我没有投票给你。我不喜欢三元组,因为尚不清楚:的两边是什么。 if-else读起来像是英语:不能错过它的意思。

– Almo
17年1月5日,19:37



FWIW我怀疑您会投票否决,因为这与梅森·惠勒的答案非常相似,但他是第一个获此殊荣的人。

–鲍勃·特威(Bob Tway)
17年1月6日于13:48

三元运算符的死亡!! (此外,除了制表符,空格,任何括号和缩进模型之外,制表符的消失(实际上,唐纳德(tm)发推文说,这将是他20日制定的前三部法律)

–莫格说要恢复莫妮卡
17年1月6日在15:37

#8 楼

与几乎所有上述答案一样,可读性应始终是您的主要目标。但是,我还认为,与创建变量和换行相比,格式化可以是一种更有效的方法。

return ((decimal)CostIn > 0 && CostOut > 0) ?
       100 * ( (decimal)CostOut - (decimal)CostIn ) / (decimal)CostOut:
       0;


在大多数情况下,我完全同意圈复杂度的论点,但是您的函数看起来既小又简单,可以用一个好的测试用例解决。出于好奇,为什么需要强制转换为小数?

评论


进行强制转换的原因是实体框架。数据库需要将它们存储为可空类型。双?在C#中不等同于Double,需要强制转换。

–鲍勃·特威(Bob Tway)
17年1月5日在21:27

@MattThrower您的意思是十进制,对吗? double!=小数,有很大的不同。

–pinkfloydx33
17年1月6日,0:16

@ pinkfloydx33是的!只用一半的大脑在电话上打字:)

–鲍勃·特威(Bob Tway)
17年1月6日在9:00

我必须向学生解释,SQL数据类型与编程语言中使用的类型奇怪地不同。不过,我无法向他们解释原因。 “我不知道!” “小端!”

–user251748
17年1月6日在14:19

我觉得这根本不可读。

– Almo
17年1月7日在14:06

#9 楼

代码被编写为人们理解。在这种情况下,简洁性并不能带来太多好处,而且会增加维护人员的负担。为简便起见,您绝对应该通过使代码更具自文档性(更好的变量名)或添加更多注释来解释为什么这样做的原因来扩展此功能。

编写代码来解决问题时今天,当需求改变时,明天的代码可能会成为问题。始终必须考虑维护,并且提高代码的可理解性至关重要。

评论


这是软件工程实践和原则发挥作用的地方。非功能性要求

– hanzolo
17年1月7日在1:00

#10 楼

在我看来,这里可读性的一个大问题是完全缺乏格式。

我会这样写: br />取决于CostInCostOut的类型是浮点型还是整数型,某些强制转换也可能是不必要的。与floatdouble不同,整数值隐式提升为decimal

评论


抱歉,我对此无可厚非,没有任何解释,但对我来说,这与Backpackcodes的回答减去他的某些言论完全相同,因此我认为这是合理的。

– PJTraill
17年1月8日在20:07

@PJTraill我一定很想念它,的确几乎一样。但是,我强烈希望在新行上添加运算符,这就是为什么我要保留我的版本的原因。

– Felix Dombek
17年1月8日在20:11



我同意运营商的意见,正如我在对其他答案的评论中指出的那样-我没有发现您按照我的意愿做了。

– PJTraill
17年1月8日在20:18

#11 楼

可以急着编写代码,但是在我的世界中,上面的代码应该用更好的变量名编写。 br />
var totalSales = CostOut;
var totalCost = CostIn;
var profit = (decimal)(CostOut - CostIn);
var grossMargin = 0m; //profit expressed as percentage of totalSales

if(profit > 0)
{
    grossMargin = profit/totalSales*100
}


评论


您失去了除以零将返回零。

–danny117
17年1月6日在17:05

这就是为什么重构为简洁起见最大化的其他人的代码很棘手的原因,为什么有额外的注释来解释为什么/如何工作是件好事

–旧帐户
17年1月7日在19:26

#12 楼

我假设CostIn * CostOut是整数
这就是我的写法
M(货币)是十进制

return CostIn > 0 && CostOut > 0 ? 100M * (CostOut - CostIn) / CostOut : 0M;


评论


希望他们不是两个消极的想法:p

–沃尔夫特
17年1月6日在13:20

除以零还在吗?

–danny117
17年1月6日在17:07

@ danny117当简洁产生错误的答案时,它就已经走了很远。

–狗仔队
17年1月6日在17:11

不想更新问题并将其激活。 100M和0M强制使用小数。我认为(CostOut-CostIn)将作为整数数学执行,然后将差值转换为十进制。

–狗仔队
17年1月8日在19:21

#13 楼






,精简不再是一种美德。
没有对空的检查。 br />尝试捕获与抛出一条可以处理错误的食物链。
假设异步任务的完成顺序
使用延迟而不是将来重新安排任务的任务
不需要已使用IO
不使用乐观更新


评论


没有足够长的时间时回答。

–user251748
17年1月6日在14:21

好的,这应该是评论,而不是答案。但是他是个新人,至少可以解释一下;不要只是投反对票并逃之!!欢迎登上,丹尼。我取消了弃权票,但是下次发表评论:-)

–莫格说要恢复莫妮卡
17年1月6日15:39



好的,我已经将答案扩展到了一些更复杂的东西,这些东西我已经学会了编写简短代码的难易方式。

–danny117
17年1月6日在17:00

感谢@Mawg的欢迎,我想指出对null的检查是我在简短代码中遇到最多问题的原因。

–danny117
17年1月6日在17:09

我只是通过Android进行了编辑,它没有要求提供编辑说明。我添加了乐观更新(检测更改并发出警告)

–danny117
17年1月9日,0:50

#14 楼

如果这通过了验证单元测试,那就很好了,如果添加了新的规范,需要新的测试或增强的实现,并且需要“整理”代码的简洁性,那就是问题就会出现。

很明显,代码中的错误表明Q / A还有另一个问题,那就是疏忽大意,因此,存在一个未被发现的错误的事实值得关注。

在处理非功能性需求(例如代码的“可读性”)时,需要由开发经理定义并由首席开发人员进行管理,并由开发人员加以尊重,以确保正确的实现。

尝试确保代码的标准化实施(标准和约定),以确保“可读性”和“可维护性”的易用性。但是,如果未定义和实施这些质量属性,那么最终将得到上面的示例所示的代码。

如果您不喜欢这种代码,请尝试使团队就实施标准和约定达成一致,并写下来,并进行随机代码审查或抽查以验证约定受到尊重。