几周前，我参加了一个软件工艺活动，评论中的一个是“我确定我们看到错误代码后，我们都可以识别”，每个人都点了点头，没有进一步讨论。

这种事情总是让我感到担忧，因为每个人都认为自己是高于平均水平的司机。尽管我认为我可以识别错误的代码，但我还是想了解更多有关其他人认为代码气味的信息，因为很少有人在博客上或仅在少数书籍中对此进行详细讨论。特别是，我想听到一种语言而不是另一种语言的代码味道会很有趣。

我将从一个简单的语言开始：


源代码管理中注释率很高的代码
代码-为什么在其中？
是要删除吗？是半成品吗？
吗？也许应该不应该
将其注释掉，而只是在有人测试某些东西时才进行
？就个人而言，我发现这种
事情确实很烦人，即使它只是在这里和那里的奇数行，但是当您看到大块散布在其余代码中时，那是完全不可接受的。它通常也表明该代码的其余部分也可能具有可疑的质量。


评论

有时我会遇到一些人，他们将代码注释掉，检入并说：“将来可能再次需要它-如果现在删除它，我会丢失它”。我必须反驳“ Er，...但这就是源代码管理的目的。”
有时，尤其是在优化时，将旧代码作为注释很方便，因此您知道模糊的优化代码将替换什么。想像一下，用一线位旋转交换替换掉三线交换并保留temp。 （尽管如此，除非程序大小至关重要，否则我认为不需要使用单行交换-EVER。）
我正在维护/清理由我们的一位工程师编写的代码，这些工程师编写了一些有用的东西，但承认他不是程序员。当我合并内容时，我将注释掉他的旧代码，然后我们回顾这些更改，并向他展示如何用更小/更有效/更容易理解的东西替换他。之后，我将那些块去除，然后将其检入。拥有旧代码很有好处，因为他看到了如何更简单地完成工作，并且我还记得为什么我在讲话时更改了事情。
我将“可能使用”的内容留给1次提交，然后，如果事情没有破裂或找不到需要，则在下一次提交时将其删除。
嗯printf（“％c”，7）通常会为我敲响警钟。 ;）

#1 楼

 /* Fuck this error */


通常在无用的try..catch块中找到，它容易引起我的注意。几乎和/* Not sure what this does, but removing it breaks the build */一样。

还有两件事：


多个嵌套的复杂if语句
用于确定的try-catch块定期执行逻辑流
具有通用名称process，data，change，rework，modify的函数

每100行有六种或七种不同的支撑样式

我刚刚发现：

 /* Stupid database */
$conn = null;
while(!$conn) {
    $conn = mysql_connect("localhost", "root", "[pass removed]");
}
/* Finally! */
echo("Connected successfully.");


对，因为必须强行使用MySQL连接用正确的方式做事。事实证明，数据库的连接数存在问题，因此它们一直处于超时状态。他们没有进行调试，而是反复尝试，直到工作为止。

评论

如果我能投票赞成6次！所有好的例子。我还不喜欢傲慢/有趣的评论（特别是如果其中包括骂人的话）-第一次阅读它们可能会有点可笑，但很快就会变得很老（因此会分散注意力）。

– FinnNk
2010-10-24 22:14
我喜欢您的示例，尽管我会说在某些情况下，不可避免要嵌套多个if语句。在具有大量业务逻辑的情况下，代码可能会有些混乱，但是如果业务本身开始令人困惑，则简化代码将降低对流程的建模的准确性。正如爱因斯坦所说：“事情应该尽可能简单，而不是简单一点。”

–摩根·赫洛克（Morgan Herlocker）
10-10-25在1:15
@Prof Plum-您能举什么例子？通常，多个嵌套if的替代方法是将其分解为（许多）方法。初级开发人员倾向于避免这种情况，好像它不如if那样可取。但通常在按下时会说“如果要减少行数”。需要对OOP有信心的人介入，并提醒他们更少的行=更好的代码。

– STW
2010-10-25 1:29
@STW这是一个好点，但是，我想说这取决于嵌套的深度。我当然同意，除了三个嵌套之外，通常还需要重构，因为它会变得很毛茸茸。但是，保险报价是一个很好的例子，其中多重嵌套实际上可以很好地模拟现实世界。除某些费率/溢价外，该手册的字面含义如下：“如果这是一项财产，并且其最低费率低于5.6，并且是在NC中，并且在房屋中有船，则应这样做并这样。”还有许多其他选择。

–摩根·赫洛克（Morgan Herlocker）
2010-10-25 1:44
@josh，如果“他们”是同事，那么我会思考您为什么不说“我们” ...

–user1249
2010-10-25 2:28

#2 楼

对我而言，主要的危险标志是重复的代码块，因为这表明该人要么不了解编程基础知识，要么就因为太害怕而无法对现有代码库进行适当的更改。

我曾经也将缺少注释视为危险信号，但是最近在处理很多非常好的代码而没有注释的情况下，我对此进行了缓解。

评论

+1：我从同事那里看到了一些代码，他们自称是linux专家，他写了一个简单的循环应用程序作为一个长函数，整个过程在main（）中一遍又一遍。 kes。

– KFro
2010-10-25 4:32
@KFro，循环展开。那就是编译器一直在做的事情:)非常高效！

–user1249
2010-10-25 6:23
@Thorbjorn：有时候，您需要稍微帮助编译器；毕竟，您是聪明的人，他只是一台笨拙的计算机。

–yatima2975
10-10-25在10:16
我见过的另一个原因：顾问被要求尽快实施该功能（当然，这也是为什么缺少测试和文档的原因）。复制/粘贴比思考如何正确做事要快。

– LennyProgrammers
2010-12-13 17:07


避免代码重复是一种困扰。在像C ++这样的语言中，要剔除各个部分并不总是那么容易，但仍然具有健壮和高效的代码。有时，一些剪切和粘贴是更实际的选择。同样，可以应用优化原则-剪切和粘贴可以为您提供快速简便的解决方案，您以后可以根据需要进行重构。您可能为以后节省了一些麻烦的维护工作，但是您可以确定自己现在正在避免延迟。

–Steve314
2010-12-23 10:02

#3 楼

尽管代码并没有增加任何实际价值，但它试图证明程序员的聪明之处：

x ^= y ^= x ^= y;


评论

哇，这比swap（x，y）更具可读性；

– JBR威尔金森
2010-11-22 15:04
如果x和y是指针，并且此分配发生在合理的时间长度内，则听起来像是破坏了Boehm GC之类的保守垃圾收集器。

–SingleNegationElimination
2010-12-13 14:56
顺便说一下，由于多次更改而中间没有插入顺序点，因此该代码在C和C ++中尚未定义。

– fredoverflow
2010-12-20 16:16
看着那，唯一想到的就是表情符号：^ _ ^

– Darien
2011年8月4日在18:05


#4 楼


20,000个（夸张的）行功能。任何占用多个屏幕的函数都需要重构。
沿着相同的代码行，类文件似乎永远存在。可能有一些概念可以抽象到类中，以清除原始类的目的和功能，以及可能在何处使用它，除非它们都是内部方法。
非描述性，非琐碎的变量，或太多琐碎的非描述性变量。这些使推论实际上是一个谜。


评论

我倾向于尽可能将功能限制为1个屏幕。

–马特·迪特罗里奥（Matt DiTrolio）
2010-10-25在16:08
1个屏幕甚至是一屏。大约十行后，我开始感到脏。

–布莱恩·罗（Bryan Rowe）
2010-10-25 20:10
好吧，我要说一下可能不受欢迎的意见。我说编写代码是原子的，从上到下的过程，这些过程被分解成单独的函数，这是代码的味道，因为开发人员坚持某些“功能应该简短”的习惯。函数应该沿功能行断开，而不仅仅是因为它们应该是一些神话般的“正确大小”。这就是为什么它们被称为“功能”的原因。

–丹·雷（Dan Ray）
2010-12-23 13:11
@Dan，功能不应该因为简短而简短，但是一次只能拥有太多信息。也许对于我来说，我的大脑很小，那限制是几个屏幕：）。在开始测试边界时，将功能分为多个功能是避免错误的必要条件。一方面，它提供了封装，因此您可以在更高的层次上进行思考；另一方面，它隐藏了正在发生的事情，因此使函数的工作更加困难。我认为应该分解功能以提高可读性，而不是为了适应“完美的长度”。

– Dominique McDonnell
2010-12-23 21:37
@ Dominic，@Péter，我认为我们三个人实际上是在说同样的话。当有充分的理由将代码分解为较小的函数时，我是支持的。我所拒绝的是为了功能设计中的短性而设计的短性。您知道，调用栈比需要的时间长三倍，但是至少这些函数很短。我宁愿调试一个可以很好且清楚地完成一件事情的高个函数，而不是通过十二个链接的函数来追逐执行路径，每个链接函数只能从前一个函数中调用。

–丹·雷（Dan Ray）
11年1月28日在13:08

#5 楼

{ Let it Leak, people have good enough computers to cope these days }


更糟糕的是，这是来自商业图书馆！

评论

这不会响起警钟。它实际上将您踢到了腿之间。

–史蒂文·埃弗斯（Steven Evers）
2010-10-27在0:17
女儿是冰毒瘾君子。谁在乎，至少她不会变得肥胖。 ：：叹：：

–伊文·普莱斯
10 Nov 19 '21：52
当我遇到麻烦时，邦图妈妈来找我。说到智慧的话，就让它泄漏。让它泄漏..让它​​泄漏。让它泄漏哦，让它泄漏。如果它只泄漏一次，那不是leaaaaaak。 （如果您到目前为止阅读过，请+1）。我真的需要考虑脱咖啡因。

– Tim Post
2010-12-13 14:57


“随着截止日期的临近，泄漏是我所能看到的，在某人耳语的地方，'写在C……eeeeee ！！！”

– chiurox
2010-12-18 14:32
从前有两个操作系统。如果运行超过49天，其中一个泄漏并坠毁，另一个则完美无缺，并且可以永久运行。一个于1995年推出，是为了吸引巨大的同志，并被数以百万计的人使用-另一个从未出货，因为他们仍在检查它是否无错误。哲学和工程学是有区别的。

–马丁·贝克特（Martin Beckett）
2010-12-18 21:52

#6 楼

注释非常冗长，以至于如果有英文编译器，它可以编译并完美运行，但不会描述代码没有的任何内容。



< pre class =“ lang-c prettyprint-override”> //Copy the value of y to temp. temp = y; //Copy the value of x to y. y = x; //Copy the value of temp to x. x = temp;

此外，如果代码遵循一些基本准则，对代码的注释也可以取消：

 //Set the age of the user to 13.
a = 13;


评论

那里叫COBOL :-)

– Gaius
2010-10-25 14:16
第二种情况不是最坏的情况。最坏的情况是：/ *将用户年龄设置为13 * / a = 18；

– PhiLho
2010-12-20 10:13


@PhiLho-不，更糟的是* /中缺少/时，因此忽略下一个* /末尾的所有代码。幸运的是，语法高亮使这种事情如今很少见了。

–Steve314
2010-12-23 10:18
再糟糕一点，一个user_age吗？真？

– glasnt
2011-3-24的3:05
我曾经在以前的雇主那里维护过代码标准文档，其中一部分是适当的注释。我最喜欢的示例来自MSDN：i = i + 1； //增加我

–迈克尔·伊佐（Michael Itzoe）
2012年11月15日14:25

#7 楼

编译时产生警告的代码。

评论

有一个候选者可以将“所有警告作为错误”编译器选项添加到makefile /项目中。

– JBR威尔金森
2010-11-22 15:07
我想如果您在一个由多个您根本不信任的人组成的项目中可能会很有用-尽管如果我要加入一个设置了该选项的项目，那将使我自己担心另一个程序员。

–宫阪丽
2010-12-10 15:22
我不同意这一点。某些编译器警告（例如，有符号的和无符号的比较，当您知道两个值都是无符号的时，即使类型不同）也比使用不必要的强制转换乱扔代码更可取。如果我使用可移植的有符号整数来减少代码，那么只有当整数具有无符号值时函数才会对其进行修改，我将这样做。

– Tim Post
2010-12-19 3:24
我宁愿用几乎多余的（无符号的int）使代码混乱，也不愿用良性警告使我的警告/错误列表混乱。我不希望警告列表成为盲点。 PITA还可以向其他人解释为什么您忽略警告，而不是解释为什么要将自然整数转换为无符号整数。

–宫阪丽
2010-12-19 18:56
有时，无论您做什么，都必须使用会产生错误的API。经典示例是在API被设计破坏的情况下定义的（一些旧的ioctl（）常量就是这样，有时OS开发人员坚持在标头中使用错误的类型）或在不推荐使用某些东西的情况下留下一个好的替代品（Apple，谢谢您）。

–研究员
2011年8月4日上午10:13

#8 楼

名称中使用数字而不是描述性名称的函数，例如：

 void doSomething()
{
}

void doSomething2()
{
}


请，使函数名称有意义！如果doSomething和doSomething2做类似的事情，请使用区分差异的函数名称。如果doSomething2是doSomething的功能突破，请为其功能命名。

评论

同样，@ Parm for SQL

–戴夫
2010-11-22 19:04
+1-输入mshtml-令我eyes然大悟:(

–凯尔·罗森多（Kyle Rosendo）
2010-12-27 7:59


GUI代码是一个例外。例如，如果一个蜗牛邮件表单有两个地址；地址1和地址2比地址和alternateAddress更合理。仅为静态的虚假标签也是一个合理的例外。

–伊文·普莱斯
2011年1月11日18:22
@Evan-相当公平，尽管我在功能上有所作为。

–旺角桑科
2011年1月11日18:25
+1-我什至看到了它用作伪版本控制方法。

– E.Z.哈特
2011年3月4日19:05

#9 楼

魔术数字或魔术字符串。

    if (accountBalance>200) { sendInvoice(...); }

   salesPrice *= 0.9;   //apply discount    

   if (invoiceStatus=="Overdue") { reportToCreditAgency(); }


评论

对于后两个而言，并没有那么糟糕，至少可以说明折扣，并且“过期”是直观的。另一方面，200 ...

–塔尔卡
2010-10-25 14:39
@Slokun-直观性不在于维护性和脆弱性。例如，考虑当折扣金额更改并且0.9在六个不同的地方硬编码时会发生什么。另外，使用字符串而不是常量/枚举在使用区分大小写的字符串的语言中会引起麻烦。

–JohnFx
2010-10-25 14:43
+1我只是花了太多时间来调试一个问题，该问题原来是由“ timeout = 15;”行引起的埋在一个程序中。

–aubreyrhodes
2010-12-13 21:35


我认为有时最后一个还可以，取决于invoiceStatus的数据来自何处。如果它只是来自返回解码后的JSON的公共api，则可以对硬编码的String常量进行检查。同意“ 200”和“ 0.9”只是魔术常数，不应以这种方式进行硬编码。即使仅在一个地方使用它们，如果在配置部分中分别定义它们而不是将它们散布在逻辑代码中，则维护起来也更加容易。如果在多个地方使用它们，维护将变得更加容易。

–李本
13年2月25日在21:30

#10 楼



也许不是最坏的情况，但清楚地显示了实现者级别：

 if(something == true)



如果语言具有for循环或迭代器构造，则使用while循环还可以演示实现者对该语言的理解水平：

 count = 0; 
while(count < items.size()){
   do stuff
   count ++; 
}

for(i = 0; i < items.size(); i++){
  do stuff 
}
//Sure this is not terrible but use the language the way it was meant to be used.


文档/注释中的拼写/语法不佳几乎等于代码本身。这样做的原因是因为代码是供人类阅读和运行机器的。这就是为什么我们使用高级语言的原因，如果您的文档难以理解，则会使我先占一席之地而对代码库形成负面看法。


#11 楼

我立即注意到的是深层嵌套的代码块（如果是，虽然是等）的频率。如果代码的深度经常超过两个或三个级别，则说明设计/逻辑问题。而且，如果它深入到8个巢穴，最好有一个很好的理由使其不被分解。

评论

我知道有人知道每种方法都应该只有一个return语句，但是当它导致6级以上的if / then嵌套时，我认为这样做的弊大于利。

– Darien
2011年8月4日在18:08

#12 楼

给学生的课程评分时，有时可以说出“眨眼”的时刻。这些是即时提示：


格式不良或不一致
连续多于两个空行
命名规范不一致或不一致
重复的代码，重复的字词越多，警告就越强烈
简单的一段代码应该过于复杂（例如，以复杂的方式检查传递给main的参数）

很少我的第一印象是不正确的，这些警告钟在大约95％的时间内是正确的。一个例外是，该语言的新学员使用的是另一种编程语言的样式。挖掘并用另一种语言重新阅读他们的风格后，我的警报声消除了，学生因此获得了充分的信誉。但是，此类异常很少见。

在考虑更高级的代码时，这是我的其他警告：


存在许多仅是“结构”的Java类。来保存数据。字段是公共字段还是私有字段，以及使用getter / setter都无关紧要，但这仍然不是经过深思熟虑的设计的一部分。
名称不佳的类，例如只是名称空间，没有代码中真正的凝聚力
参考代码中甚至没有使用的设计模式
没有解释的空异常处理程序
当我在Eclipse中编写代码时，会有数百个黄色的“警告”将代码排成一行，主要是由于未使用的导入或变量

在样式方面，我通常不喜欢看到：代码

这些只是错误代码的线索。有时看似不好的代码实际上不是，因为您不了解程序员的意图。例如，可能有一个很好的理由，即某些事情看起来过于复杂-可能还有其他考虑因素在起作用。

评论

我看不到从一种语言到另一种语言使用样式是多么严重的错误（每个缩进2、4、8个空格...）。如果学生没有其他可遵循的风格，那么自洽就没有错。作为分级员，您会看到十亿个程序，因此您处于另一端，但这并不是完全摒弃不同（但一致）风格的原因。

–尼克T
10-10-25在16:37


我发现仅聚合数据的类-结构即没有问题。这就是数据传输对象（DTO）的功能，并且可以使代码的可读性比说的更好，例如将20个参数传递给方法。

–内米
2010-10-26 17:14


您对struct的评论已到位。如果数据为原始格式，则不会有任何修改，这很好。但是95％的时间里，您应该在类中具有一些函数来格式化/操作数据。我只记得我的一些代码基本上使用了这种模式，应该加以改进。

–心怀不满的山羊
2010-10-27 14:42


+1表示不一致的样式和多余的换行符（我见过随机的缩进，没有缩进，随机的和不断变化的命名约定以及更多-以及在生产代码中！）。如果您什至不愿意将其正确处理，那么您还能做什么呢？

–迪恩·哈丁（Dean Harding）
10-10-28在5:19
我寻找相对于主体缩进得太远的方法的声明行。这是他们从其他人的文件中复制粘贴的标志。

–巴里·布朗（Barry Brown）
2010-11-20 23:00

#13 楼

个人喜好/宠爱：IDE生成的名称已被使用。如果TextBox1是系统中的主要变量，那么代码审查将使您有另一件事。

#14 楼

完全未使用的变量，尤其是当变量的名称与所使用的变量名称相似时。

#15 楼

很多人提到：



 //TODO: [something not implemented]


而我希望这些东西得到实施，至少他们做了一个说明。我认为更糟糕的是：

 //TODO: [something that is already implemented]


如果您从不打扰的话，Todo毫无价值，而且令人困惑删除它们！

评论

我只是经历了必须在我们的发行产品中生成所有TODO的报告以及执行处置它们的计划的过程。近一半被淘汰。

– AShelly
10-10-26在19:11
-1 TODO注释在MS Visual Studio中用于跟踪项目中仍需要工作的部分。 IE，IDE保留了一个跟踪TODO的列表，因此您可以轻松地单击它们并将其直接带到TODO所在的行。我希望看到TODO明确放置，以查看仍需要完成哪些工作。请参阅dotnetperls.com/todo。

–伊文·普莱斯
10 Nov 19 '21：49
@Evan Plaice：哇，您的意思是说VS IDE可以在实现某些功能后识别并删除// TODO注释？太棒了！

– JBR威尔金森
2010-11-22 15:06


@Prof Plum为什么不创建一个政策，将TODO负责人的姓名放在注释中。这样，如果有剩菜

–伊文·普莱斯
2010-11-25 2:37
比// TODO更好的计划，使用您的Bug跟踪器，这就是它的目的！

–SingleNegationElimination
2010-12-13 14:58


#16 楼

需要我向下滚动才能阅读所有内容的方法。

评论

嗯..这取决于正在实施的内容。当实现一些复杂的算法时，发生这种情况并非没有道理，因为这是它们的定义方式。当然，如果大多数方法都是那样，那就是一个危险信号。

–比利·奥尼尔（Billy ONeal）
10-10-24在23:51
作为总括性的说法，我不同意，浪费时间不断进行重构，以使所有内容都符合这种自我施加的规则，实际上会增加项目的总体成本。

–匿名类型
2010-10-25 2:48
我不同意该规则会增加项目的总体成本，但是我认为这是主观的，因为它取决于开发人员。如果在开发时遵循“关注点分离”的一般原则，那么如果选择这样做，那么重构将不是一件容易的事。需要考虑的一点是，如果没有编写原始项目的开发人员试图用300多个代码行来修正一堆方法，那么三年下来要花多少钱？费用多少？

–布拉德
2010-10-25 8:18


向右滚动比向下滚动更让我烦恼。空格是“免费的”。

–旺角桑科
2010-10-25 13:37
我宁愿滚动，也不必跳过整个文件（或多个文件）以弄清楚代码在做什么。

– TMN
2010-10-25 17:15

#17 楼

方法名称中的连接词：

 public void addEmployeeAndUpdatePayrate(...) 


public int getSalaryOrHourlyPay(int Employee) ....


说明：引起警报的原因是它表示该方法可能违反了单一责任原则。

评论

嗯...如果函数名称准确地定义了函数的功能，那么我不同意。如果该方法将两件事分开做，最好将它们分开，那么我可能会同意，具体取决于上下文。

–旺角桑科
10-10-25在15:07
这才是重点。连词暗示它很可能做不止一件事情。根据这个问题，这只是使我意识到某些事情可能是错误的事情。

–JohnFx
2010-10-25 15:23
现在，如果您必须在多个地方添加雇员并更新他们的薪水，该怎么办？如果该方法包含两个方法调用（addEmployee（）; updatePayrate（）;），那么我认为这不是问题。

–马特格兰德
2012-2-27在16:36

#18 楼

将明显地GPL的源代码链接到一个商业的，封闭源代码的程序中。在代码中也是如此。

评论

尽管您的观点很好，但您的语气是不必要的。

– JBR威尔金森
2010-11-22 15:10
@JBRWilkinson：谢谢，你是对的。我向所有人道歉。

–鲍勃·墨菲（Bob Murphy）
10 Nov 23'0:03
我认为您的标题需要重写。静态和动态链接到GPL的源代码都违反了GPL ...

–加文·科茨（Gavin Coates）
2011年8月4日在8:45
好点。我已经重写了整个帖子。谢谢大家。

–鲍勃·墨菲（Bob Murphy）
2011年8月4日在17:02

#19 楼

与语言无关：


TODO: not implemented

int function(...) { return -1; }（与“未实现”相同）
由于非异常原因而引发异常。
/>误用或不一致使用0，-1或null作为特殊的返回值。
断言中没有令人信服的注释，说明为什么它永远不会失败。

语言特定（C ++）：


C ++小写宏。
静态或全局C ++变量。
未初始化或未使用的变量。
任何array new显然都不符合RAII安全。
>任何显然不是边界安全的数组或指针的使用。这包括printf。
任何使用数组的未初始化部分。

Microsoft C ++特定于：


与标识符冲突的任何标识符名称
通常，对Win32 API的任何使用都是警钟的重要来源。始终打开MSDN，并在有疑问时查找参数/返回值定义。 （已编辑）


特定于C ++ / OOP：


实现（具体类）继承，其中父类同时具有虚拟和非虚拟方法，在应该/不应该虚拟之间没有明显的概念上的区别。


评论

// TODO：对此答案发表评论

– Johnc
2010-10-25 8:55


我猜“不可知的语言”现在意味着“ C / C ++ / Java”吗？

– Inaimathi
2010-10-25 14:51
+1“因非异常原因而引发异常”完全不同意！

–billy.bob
2010-10-26 8:36
@Inaimathi-TODO注释，函数存根，异常滥用，零/空/空语义的混淆以及毫无意义的健全性检查是所有命令式/ OOP语言以及在某种程度上通常是所有编程语言固有的。

–宫阪丽
2010-10-28 4:17
我相信小写的C预处理程序宏是可以的，但前提是它们仅对参数进行一次评估并且仅产生一条语句。

–乔D
2011年1月2日在22:49


#20 楼

使用大量文本块而不是枚举或全局定义的变量。

不好：



 if (itemType == "Student") { ... }


更好：

 private enum itemTypeEnum {
  Student,
  Teacher
}
if (itemType == itemTypeEnum.Student.ToString()) { ... }


最佳：

 private itemTypeEnum itemType;
...
if (itemType == itemTypeEnum.Student) { ... }


评论

或最佳：使用多态性。

–Lstor
2011年8月4日在7:52

#21 楼

奇异的压痕样式。

有两种非常流行的样式，人们将把这种争论推向高潮。但是有时候我看到有人使用一种非常罕见的，甚至是一种自家的缩进样式。这表明他们可能没有与自己以外的任何人进行编码。

评论

或只是表明他们是一个非常珍贵的个人主义才能，还没有被与“最佳实践”无关的同质化编码实践所吸引。

–匿名类型
2010-10-28 22:35
我希望你在讽刺。如果有人以这种不寻常的方式编码，那应该引起警钟。它们可以随心所欲地具有艺术性，但仍然...警钟。

–肯
2010-10-29 2:23
尽管在Haskell / ML / F＃之外极为罕见，但我发现有一种非常罕见的样式（我相信它称为Utrecht样式）非常有用。向下滚动到“模块形状”：learnyouahaskell.com/making-our-own-types-and-typeclasses。这种样式的优点在于，您不必修改前一行的定界符即可添加新的定界符-我经常忘记这样做。

–宫阪丽
2010-12-14在1:29
@ReiMiyasaka已经晚了七年，但是...乌特勒支的风格真的让我很生气。我认为，在Haskell规范中不对垂直组织的列表强加另一个“布局规则”是一个错误。这样，解析器仅通过检查缩进就可以检测到新的列表条目，这就是每个人反正组织其列表的方式。

–赖安·赖希（Ryan Reich）
17年7月29日在19:18
@RyanReich Weird，七年后，我仍然喜欢它。不过，我同意。对于所有语法上的尴尬和失败，F＃还允许仅用换行符和缩进符（大多数情况下）来分隔项目，这使得代码整洁。

–宫阪丽
17年8月1日在23:11

#22 楼

类型不足的参数或方法上的返回值。

 public DataTable GetEmployees() {...}

public DateTime getHireDate(DataTable EmployeeInfo) {...}

public void FireEmployee(Object EmployeeObjectOrEmployeeID) {...}


评论

+1：我必须使用一些“ REST” Web服务，这些服务以伪HTML表形式返回所有内容，即使您传递的是明显的语法错误。非授权？输入完整的垃圾？服务器容量过大？ 200（在一列，一个行表中加上可怕的消息）。 AAaaaaaaaargh！

–研究员
2011年8月4日在10:30

#23 楼


多个三元运算符串在一起，因此它不再像if...else块，而是变成了if...else if...[...]...else块。
没有下划线或驼峰的长变量名。我从一些代码中获取了示例：$lesseeloginaccountservice

文件中的几百行代码，几乎没有注释，并且代码非常不明显。
if语句过于复杂。来自以下代码的示例：if (!($lessee_obj instanceof Lessee && $lessee_obj != NULL))，我将其压缩为if ($lessee_obj == null)



#24 楼

代码气味：不遵循最佳实践


这种事情总是让我感到担忧，因为有人认为每个人都认为自己是中等水平的驱动力。


这是您的新闻快讯：全球50％的人口智力水平低于平均水平。好的，所以有些人的智力完全是平均的，但是我们不要挑剔。此外，愚蠢的副作用之一是您无法识别自己的愚蠢！如果结合使用这些语句，情况看起来就不会那么好。


在查看代码时，哪些事情立即响起​​了警钟？


事情已经提到过了，总的来说，似乎没有遵循最佳实践是一种代码味道。很多时候它可能是主观的，但以我的经验，它们大多数都是合理的。遵循最佳做法应该不是问题，如果您想知道为什么会这样，请研究它而不是忽略和/或抱怨它-也许有道理，也许不是。

最佳实践的一个示例可能是在每个if块中使用curly，即使它仅包含一行：

 if (something) {
    // whatever
}


您可能没有必要，但我最近读到它是错误的主要来源。在Stack Overflow上也讨论了始终使用方括号，并且检查语句是否具有方括号也是PMD（Java的静态代码分析器）中的一项规则。请记住：“因为这是最佳实践”从来没有一个可接受的答案回答“为什么要这样做？”如果您无法阐明为什么某事是最佳做法，那么这不是最佳做法，这是一种迷信。

评论

这可能很花哨，但我认为您选择的平均水平很重要。据我了解，世界人口的50％低于智力中位数（根据定义）；但其他平均值却无法以相同的方式运作。例如，取算术平均值为2的总体（1、1、1、5）。

–烈火企鹅
2010-10-25 13:33
嗯，您引用了“ what-superstitions-do-programmers-have”一文，其中用户对没有花括号的花括号做了一个大胆的声明。我认为这并不是最佳做法的一个很好的例子。

–伊文·普莱斯
2010年11月19日在22:33
@Evan：是的，你是对的。我对此添加了更多信息，希望对您有所帮助。

–车
2010-11-20 10:58
不利的一面是那些热心遵循“最佳实践”的人，而对为什么某事物是“最佳实践”却没有任何批判性的思考。这就是为什么我强烈不喜欢“最佳实践”一词的原因，因为对于某些人来说，这是一个停止思考并追随人群的借口。对于“为什么要这样做？”这个问题，“因为这是最佳实践”绝不是可接受的答案。如果您无法阐明为什么某事是最佳做法，那么它不是最佳做法，那是一种迷信。

–丹·代尔
2010-12-18 10:43
很好的评论，丹！我在答案中添加了最后两行。

–车
2010-12-20 9:38

#25 楼

注释说：“这是因为froz子系统的设计完全令人厌烦。”

在整个段落中都这样。

他们解释说需要进行以下重构。

但是没有这样做。

现在，他们可能被告知他们不能不能因为时间或能力方面的问题而由当时的老板来更改它，但这也许是因为人们太小气了。

如果主管认为j.random。程序员无法进行重构，那么主管应该去做。

无论如何，我知道代码是由一支分散的团队编写的，具有可能的权力策略，并且他们没有因重构而感到厌烦子系统设计。

真实的故事。可能会发生在你身上。

#26 楼

谁能想到一个示例，其中代码应通过绝对路径合法地引用文件？

评论

XML模式算不算？

–尼克T
2010-10-25 16:45
系统配置文件。通常应通过./configure进行设置，但即使这样也需要在某个地方使用默认值。

–eswald
2010-10-25 16:54
/ dev / null和朋友都可以。但是，即使像/ bin / bash这样的东西也值得怀疑-如果您是一个具有/ usr / bin / bash的怪异系统，该怎么办？

–汤姆·安德森（Tom Anderson）
2010-10-26 13:40
由JAX-WS工具（至少为JBossWS和Metro）生成的Web服务客户端代码包含WSDL文件的硬连线绝对路径（两次！）。这可能非常不合适，例如/home/tom/dev/randomhacking/thing.wsdl。这是默认行为，这在犯罪上是疯狂的。

–汤姆·安德森（Tom Anderson）
2010-10-26 13:43
关于/ dev / null：在Windows上进行开发时，我有将应用程序和库保存在c：\ dev下的习惯。不知何故，总是在该文件夹内自动创建一个文件夹null。我发誓我不知道是谁做的。 （我最喜欢的错误/功能之一）

–塞恩·帕特里克·弗洛伊德（Sean Patrick Floyd）
2010-11-22 14:15

#27 楼

捕获一般异常：



 try {

 ...

} catch {
}


或

 try {

 ...

} catch (Exception ex) {
}


区域过度使用

通常，使用太多区域对我来说意味着你的班级太大了。这是一个警告标志，表示我应该进一步研究这段代码。

评论

仅当您不执行任何操作就将其抛出时，捕获常规异常才是问题。实际上，对于大多数事情来说，一个异常类就足够了。我倾向于使用runtime_error。

– CashCow
2011年3月2日在13:18
对于“捕获并丢弃异常”示例为+1。如果您没有执行任何例外操作，请不要抓住它。至少要记录下来。至少，至少要添加注释，以解释为什么可以捕获所有异常并在代码的这一点继续前进。

– E.Z.哈特
2011年3月4日19:09

#28 楼

类命名约定表明对它们试图创建的抽象的理解不足。还是根本就没有定义一个抽象。

我想到了一个极端的例子，在一个VB类中，我看到有一次名为Data，长30,000+行...在第一个文件中。这是一个局部类，分为至少六个其他文件。大多数方法都是使用诸如FindXByYWithZ()之类名称存储过程的包装器。

即使没有那么生动的示例，我也确信我们都已经将逻辑“转储”到了一个构思不佳的类中，完全通用的标题，后来又后悔了。

#29 楼

重新实现语言基本功能的功能。例如，如果您在JavaScript中看到“ getStringLength（）”方法而不是对字符串的“ .length”属性的调用，则说明您遇到麻烦了。

#30 楼

 #define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...
#define ...


当然没有任何文档，并且偶尔还有嵌套的#define s

评论

昨天，我在生产代码中完全看到了这种“模式”……甚至在C ++生产代码中更糟……：/

–奥利弗·韦勒
2010-12-18 10:31