我编写了以下代码:
if (boutique == null) {
boutique = new Boutique();
boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setSelected(false);
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());
boutiqueDao.persist(boutique);
} else {
boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
//boutique.setSelected(false);
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());
boutiqueDao.merge(boutique);
}
这里有一个注释行。但是我认为通过使
if
和else
之间的区别显而易见,可以使代码更清晰。颜色突出显示之间的区别更加明显。注释掉这样的代码是否是个好主意?
#1 楼
大多数答案都集中在如何重构这一特定情况上,但是让我对为什么注释掉的代码通常不好的问题提供一个一般性的答案:首先,注释掉的代码不会编译。这很明显,但这意味着:
该代码甚至可能无法工作。
注释的依赖项更改时,显然不会中断。
注释的代码非常“死代码”。它坐在那里的时间越长,它旋转的时间就越长,并为下一个开发人员提供越来越少的价值。
其次,目的尚不清楚。您确实需要更长的注释,以提供有关为何存在随机注释行的上下文。当我只看到注释的代码行时,我必须研究它如何到达那里,只是为了了解为什么到达那里。谁写的?什么犯?提交消息/上下文是什么? Etcetera。
考虑替代方案:
如果目标是提供使用函数/ api的示例,请提供单元测试。单元测试是真实的代码,当它们不再正确时会中断。
如果目的是保留代码的先前版本,请使用源代码控制。我宁愿签出以前的版本,然后在整个代码库中切换注释以“还原”更改。
如果要维护同一代码的替代版本,请再次使用源代码控制。毕竟,这就是分支的目的。
如果目的是澄清结构,请考虑如何重组代码以使其更明显。其他大多数答案都是如何执行此操作的好例子。
#2 楼
此代码的最大问题是您重复了这6行。一旦消除了重复,该注释就没有用了。如果创建
boutiqueDao.mergeOrPersist
方法,则可以将其重写为:if (boutique == null) {
boutique = new Boutique();
boutique.setSelected(false);
}
boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());
boutiqueDao.mergeOrPersist(boutique);
代码创建或更新某个对象是很常见的,因此您应该解决一次,例如,通过创建
mergeOrPersist
方法。对于这两种情况,您当然不应该重复所有分配代码。许多ORM都以某种方式内置了对此的支持。例如,如果
id
不为零,他们可能会创建一个新行,如果id
不为零,他们可能会更新一个现有行。确切的形式取决于所讨论的ORM,并且由于我不熟悉您所使用的技术,因此我无法为您提供帮助。如果您不这样做,如果要创建
mergeOrPersist
方法,则应以其他方式消除重复,例如通过引入isNewBoutique
标志。这可能并不漂亮,但是它比复制整个分配逻辑要好得多。bool isNewBoutique = boutique == null;
if (isNewBoutique) {
boutique = new Boutique();
boutique.setSelected(false);
}
boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE + fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());
if (isNewBoutique)
boutiqueDao.persist(boutique);
else
boutiqueDao.merge(boutique);
#3 楼
这是一个绝对令人恐惧的想法。尚不清楚目的是什么。开发人员是否错误地注释掉了这一行?要测试什么?除了我看到的6条线在两种情况下都是绝对相等的事实之外。相反,您应该防止此代码重复。这样一来,您会更清楚地调用setSelected。
评论
同意我假设看到注释掉的行是已删除的旧行为。如果需要注释,则注释应使用自然语言,而不是代码。
–法律
2013年3月12日在18:29
我完全同意!最近,我花了数小时试图理解和清理我继承的一些应用程序,这些应用程序由于这种做法几乎完全不可读。它还包括已与所有其他代码断开连接但未被删除的代码!我相信这是版本控制系统背后的主要目的。它具有注释以及随之而来的更改。最后,由于这种做法,我至少要增加2个星期的工作量。
–bsara
13年3月15日在3:06
在这篇文章中类似的观点:不要用注释掉的代码来污染代码库
–尼克·阿列克谢耶夫(Nick Alexeev)
18年1月15日在1:19
#4 楼
不,这是一个糟糕的主意。基于那段代码,我想到了以下想法:此行已被注释掉,因为开发人员正在调试它,却忘记将行恢复为以前的状态
此行已被注释掉,因为它曾经是业务逻辑的一部分,但现在不再适用了
此行已注释掉,因为它在生产中造成了性能问题,并且开发人员希望了解其影响生产系统
看到成千上万行注释掉的代码后,我现在正在做的唯一明智的事情是:立即删除它。
没有明智的理由将注释掉的代码检入到存储库中。
此外,您的代码使用了大量重复项。我建议您尽快对其进行优化,以提高可读性。
评论
但是我想摆脱了重复的代码,我认为这几乎不能看作是一种优化。
– Alexis Dufrenoy
13年3月11日在9:49
这是对人类可读性的优化
– jk。
13年3月11日在10:23
@Traroth,您可以针对速度,内存使用,功耗或任何其他指标进行优化,因此我看不到您无法针对可读性进行优化(尽管作为指标,它有点笨拙)
– jk。
13年3月11日在10:44
确实,我的意思是人类可读性。这里的一个小提示:编程中最重要的责任就是代码。因此,少即是多。
–迪贝克
13年11月11日在13:23
c2.com/cgi/wiki上也将软件视为责任。SoftwareAsLiability从那里开始:“产生更多的代码并不总是有好处的。代码的测试和维护成本很高,因此如果可以用更少的代码来完成相同的工作,是一个加号。请不要注释掉无效的代码,只需将其删除即可。注释掉的代码很快就会变得过时且无用,因此您最好尽快删除它,以免造成混乱。请保留良好的备份以使其更容易。”
– ninjalj
2013年3月11日23:53
#5 楼
我想补充指出CodesInChaos的答案,您可以将其进一步重构为小的方法。通过合成共享通用功能可避免出现以下条件:function fill(boutique) {
boutique.setSite(site);
boutique.setUrlLogo(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getLogo());
boutique.setUrlBoutique(CmsProperties.URL_FLUX_BOUTIQUE+fluxBoutique.getUrl());
boutique.setNom(fluxBoutique.getNom());
boutique.setIdWebSC(fluxBoutique.getId());
boutique.setDateModification(new Date());
}
function create() {
boutique = new Boutique();
fill(boutique);
boutique.setSelected(false);
return boutiqueDao.persist(boutique);
}
function update(boutique) {
fill(boutiquie);
return boutiquieDao.merge(boutique);
}
function createOrUpdate(boutique) {
if (boutique == null) {
return create();
}
return update(boutique);
}
评论
我认为这是最干净的建议。
– Alexis Dufrenoy
13年3月11日在15:07
+1,我还要补充一点,越避免绕过空对象越好(我发现此解决方案是一个很好的例子)。
– Nadir Sampaoli
2013年3月12日9:00
我会把BoutiqueDao作为创建和更新的输入。
–快乐的绿色孩子午睡
2013年3月12日17:56
这如何工作?您如何知道何时调用创建和何时调用更新?原始代码查看精品店,并知道是否需要更新或创建。直到您调用create或update为止,这只是无济于事。
–里里昂
13年3月13日在14:36
Lyrion:琐碎,为清楚起见,我还将添加该代码。
–亚历山大·托斯汀
13年13月13日在15:51
#6 楼
虽然这显然不是注释掉代码的好案例,但我认为有必要保证这种情况:// The following code is obvious but does not work because of <x>
// <offending code>
<uglier answer that actually does work>
这是对以后看到它的人的警告,即显而易见的是改进不是。
编辑:我说的是小事。如果太大,请改为解释。
评论
//接下来的部分是怎么回事,因为X?解释为什么您以自己的方式做某件事,而不是为什么您没有以某种特定方式做某件事。在您的特定示例中,它完全不需要潜在的大块注释代码。 (我没有投票,但可以肯定为什么会投票。)
–用户
2013年3月12日15:27
迈克尔(Michael),因为它使其他编码人员(和您自己,数天,数周,数月后)清楚地知道,是的,您确实尝试了这种更简洁/更聪明的方法,但是不,由于X,它没有用,所以他们不应该这样做麻烦再试一次。我认为这是一种完全合法的方法,并且对此可悲的答案持反对态度。
–加里特·奥尔布赖特(Garrett Albright)
13年3月13日在6:01
@GarrettAlbright:谢谢,我很高兴看到有人得到它。
–Loren Pechtel
2013年3月13日19:31
@LorenPechtel:不仅如此,我写的或多或少完全一样。在某些情况下,快速了解哪些“显而易见的”解决方案已被尝试而没有成功以及为什么它们不起作用非常非常有用。
– JensG
2014年4月5日在19:24
除了带有解释的失败代码外,我还将注释掉代码的替代实现,这些实现在不同的生产环境中可能更有效。例如,我已经编码了算法的简单指数时间版本和复杂的多项式时间版本。但是在当前的生产中,n很小,而指数算法要快得多。如果以后有所改变,将来跟在我项目上的开发人员将如何知道深藏在源代码控制中数百个提交中的代码的不同实现?
– Moobie
'18 Sep 5'在18:18
#7 楼
在此特定示例中,我发现注释掉的代码非常含糊,主要是因为Dibkke的答案中列出了原因。其他人提出了一些方法,您可以重构代码,甚至避免这样做的诱惑,但是,如果由于某种原因(例如,行相似,但距离不够近)而无法做到这一点,我将喜欢以下注释://无需取消选择这家精品店,因为[无论如何]
但是,我认为在某些情况下会离开(甚至添加注释掉的代码是不可理解的。当使用MATLAB或NumPY之类的东西时,通常可以编写等效的代码,这些代码要么1)遍历一个数组,一次处理一个元素,要么2)一次操作整个数组。在某些情况下,后者要快得多,但也很难阅读。如果将某些代码替换为等效的矢量化代码,则会将原始代码嵌入附近的注释中,如下所示:
%%下面的矢量化代码可以做到这一点:
% for ii in 1:N
% for jj in 1:N
% etc.
%,但矩阵版本在典型输入(MK,2013年10月10日)上的运行速度快约15倍。 />显然,需要注意两个版本实际上做同样的事情,并且注释要么与实际代码保持同步,要么在代码更改时被删除。显然,有关过早优化的常见警告也适用于...
评论
“显然,需要注意两个版本实际上做同样的事情,并且注释必须与...保持同步。”-在那儿,您解释了为什么这不是一个好主意。
–sleske
2014年8月27日在14:38
好吧,所有评论都存在这个问题,对吧?一些矢量化代码非常不透明,以至于注释是值得的,并且具有“展开”版本可以很方便地进行调试。
–马特·克劳斯(Matt Krause)
2014年8月27日在17:07
真正。不过,我会尽量使注释简短,而不要使用完整的源代码。无论如何,如果您有一个示例,询问如何使其更具可读性将是一个好问题(在此处或在codereview.s.e上)。
–sleske
2014年8月28日下午5:25
在您的最后一种情况下,我会将两个代码变体都保留为可编译代码。
– CodesInChaos
2015年2月9日在16:27
#8 楼
我唯一看到注释掉的代码有用的是在配置文件中,其中提供了每个选项的代码,但是注释掉了,只需删除注释标记就可以轻松启用设置:## Enable support for mouse input:
# enable_mouse = true
在这种情况下,注释掉的代码有助于记录所有可用选项以及如何使用它们。在整个过程中也都使用默认值,因此该代码也记录了默认设置。
#9 楼
一般而言,代码仅是编写代码的人的自我证明。如果需要文档,请编写文档。指望新来的源代码开发人员会坐下来阅读数千行代码,试图从高层次了解正在发生的事情,这是无法接受的。在这种情况下,注释掉的代码行的目的是显示两个重复代码实例之间的差异。与其尝试用注释巧妙地记录差异,不如重写代码以使其有意义。然后,如果您仍然有必要对代码进行注释,请编写适当的注释。
评论
这完全正确。许多人(包括我自己在内)认为他们的代码太棒了,以至于不需要文档。但是,除非有完整的文档和注释,否则世界上其他所有人都会发现它。
–瑞安·阿莫斯(Ryan Amos)
13年3月12日在3:04
“代码仅是编写代码的人的自我证明”-请选择您一年前编写的一段复杂的,未注释的代码,并尝试在有限的时间内理解它。你不能吗哎呀
– JensG
2014年4月5日在19:26
我认为这有些微妙。许多编写良好的代码是可理解的,无需注释即可理解。问题在于,当您只需要复杂的细节时,便试图找出更大的图景(甚至在相当本地的水平)。注释可以很好地解释非显而易见的代码块,但是当您有很好的文档字符串,解释每个函数,类和模块的实际用途时,则不需要太多帮助来理解实现。
–卡尔·史密斯
19年7月8日在18:02
#10 楼
不,注释的代码会过时,比不值钱的代码更糟,它经常有害,因为它会巩固实现的某些方面以及所有当前的假设。注释应包括接口详细信息和意图功能; “预期功能”:可以包括,首先我们尝试一下,然后尝试一下,然后我们以这种方式失败。
我见过的程序员试图将内容留在注释中就是爱上了什么他们已经写过,即使它没有在成品中添加任何东西,也不想丢失它。
#11 楼
在极少数情况下可能会发生,但并非如您所愿。其他答案已经很好地阐明了原因。罕见的情况之一是模板RPM规范,我们在我的商店中使用它作为所有新软件包的起点,主要是为了确保没有任何东西重要的被忽略了。大多数(但不是全部)软件包的压缩包中包含标准名称的源代码,并带有标签:
Name: foomatic
Version: 3.14
...
Source0: %{name}-%{version}.tar.gz
对于没有源代码的软件包,我们将其注释掉标签,并在标签上方加上另一条评论以保持标准格式,并指示有人在开发过程中已停止并考虑该问题:
Name: barmatic
Version: 2.71
...
# This package has no sources.
# Source0: %{name}-%{version}.tar.gz
不要添加您不会使用的代码,因为正如其他人所介绍的那样,它可能会误认为属于该代码的内容。它可以。但是,添加注释以解释为什么可能缺少代码的原因很有用:
if ( condition ) {
foo();
// Under most other circumstances, we would do a bar() here, but
// we can't because the quux isn't activated yet. We might call
// bletch() later to rectify the situation.
baz();
}
评论
可以说,注释没有被注释掉。
– jk。
13年3月11日在15:48
@jk:你可以说是正确的。
– Blfl
2013年3月11日15:53
#12 楼
应用程序未使用注释掉的代码,因此需要附加注释以说明为什么不使用它。但是在这种情况下,有些情况下注释掉的代码可能会有用。我想到的是,您使用一种通用且有吸引力的方法来解决问题的情况,但事实证明,您实际问题的要求与该问题稍有不同。特别是如果您的需求实际上需要更多的代码,则维护人员使用旧方法“优化”代码的诱惑可能会很强烈,但这样做只会将错误带回来。在注释中保留“错误的”实现将有助于消除这种情况,因为您可以使用它来确切说明这种方法在这种情况下是错误的。
我无法想象这种情况经常发生。通常,在不包含示例“错误”实现的情况下进行解释就足够了。但是我可以想象这还不够的情况,所以既然问题在于它是否有用,是的,它可以。只是不是大部分时间。
评论
抱歉,我看不到注释代码的任何价值。未使用注释掉的代码,因此在生产代码中没有位置。
–弗拉基米尔·科赞奇克(Vladimir Kocjancic)
2014年4月5日在19:08
请定义“二手”。
– JensG
2014年4月5日19:30在
我认为他的意思是“被处决”
– Alexis Dufrenoy
19年1月3日在10:12
#13 楼
这个伙伴看起来不太好。注释的代码是...只是代码。代码用于逻辑的实现。使代码本身更具可读性是一门艺术。正如@CodesInChaos已经暗示的那样,重复的代码行并不是很好的逻辑实现。
您真的认为一个真正的程序员会更喜欢可读性而不是逻辑实现。 (顺便说一下,我们在逻辑表示形式中添加了注释和“补语”)。
就我而言,应该为编译器编写代码,这很好-如果“它”理解码。对于人类可读性,注释对于开发人员(从长远来看),对于重用该代码的人员(例如测试人员)来说都是很好的。
否则,您可以在此处尝试更灵活的方法,例如
boutique.setSite(站点)可以替换为
setsiteof.boutique(站点)。 OOP有不同的方面和观点,通过它们可以提高可读性。
虽然这段代码乍一看似乎很吸引人,但人们可以认为他找到了一种人类可读性的方法,而编译器也确实做到了它的工作完美无缺,我们所有人都将开始遵循这种做法,这将导致模糊文件,随着时间的流逝,该文件的可读性将越来越差,并且变得越来越复杂。
评论
“就我而言,应该为编译器编写代码。”哦,请不要。这样一来,您最终会遇到怪兽,看起来像它们可以直接从混淆C竞赛之类的东西中获取。计算机是二进制的,而人类则使用模糊逻辑(顺便说一下,这对宠物主人来说是两倍)。今天,计算机时间几乎是免费的(基本上只是电力消耗),而程序员的时间则相对非常昂贵。为人类编写代码,编译器将理解它。在不考虑人类的情况下为编译器编写代码,您不会在团队中结交很多朋友。
–用户
2013年3月12日15:24
“为编译器编写代码”-实际上您不知道。您应该记住的人是负责维护您的代码的人。
– JensG
2014年4月5日19:29
评论
我认为您缺少一个重要原因:文档:如果目的是记录替代设计方案,则应提供替代方案的说明,尤其是为什么要替代替代方案,而不是原始代码。
–莎琳
13年4月5日在8:22
用编程语言比用编程语言更好地解释设计选项。
– Mark E. Haase
2014年4月5日17:50
后续接管我的项目的开发人员如何知道源代码控制中存在替代/先前/失败的实现?是否期望新开发人员浏览所有版本历史并更改日志?或者,对于每个有用的替代实现,通常都使用注释来链接到先前提交的哈希值?如果是这样,我从来没有注意到。
– Moobie
18-09-5在17:59
但是有一个警告。有时,两种等效的代码方法可能在性能和可靠性上有所不同,一种方法是高性能的,另一种是可读的。在这种情况下,可以使用Performanceant变体,但是将可读性变体放在注释中,这样可以更容易理解代码的目的。有时,(带注释的)代码行可能比冗长的解释更清晰。
–更
19年7月9日在8:41
@Flater有趣的一点,但是当代码演化时会发生什么?我很难想象开发人员会维护注释掉的代码。
– Alexis Dufrenoy
19年11月18日在9:16