当您遇到哪些类型的编码反模式时，您总是会重构它们？

Question

我只是重构了我正在处理的类的不同部分中的一些代码，因为它是一系列嵌套的条件运算符 (?:)，通过相当简单的 switch 语句 (C#) 使其变得更加清晰。

你什么时候会接触那些不是你正在直接处理的代码以使其更清晰？

Answer 1

删除/更新明显错误或明显毫无意义的评论。
删除它们是：

1. 安全的
2. 版本控制意味着您可以再次找到它们，
3. 从而提高了他人和您自己的代码质量。

这是我所知道的唯一 100% 无风险的重构。

请注意，在诸如 javadoc 注释之类的结构化注释中执行此操作是另一回事。 更改这些并不是没有风险的，因此它们很可能被修复/删除，但不能像标准的错误代码注释那样保证修复。

Answer 2

我曾经在重构时遇到过这样的代码：

string strMyString;
try
{
  strMyString = Session["MySessionVar"].ToString();
}
catch
{
  strMyString = "";
}

Resharper 指出 .ToString() 是多余的，所以我把它去掉了。 不幸的是，这最终破坏了代码。 每当 MySessionVar 为 null 时，它就不会导致 NullReferenceException，而代码正是依靠 NullReferenceException 将其降至 catch 块。 我知道，这是一些悲伤的代码。 但我确实从中吸取了很好的教训。 不要依赖工具来快速浏览旧代码来帮助您进行重构 - 自己思考一下。

我最终将其重构如下：

string strMyString = Session["MySessionVar"] ?? "";

更新：由于这篇文章正在被投票，并且从技术上讲不包含问题的答案，我认为我实际上应该回答这个问题。 （好吧，这让我很困扰，以至于我真的在做梦。）

就我个人而言，在重构之前我会问自己几个问题。

1) 系统是否受到源代码控制？ 如果是这样，请继续进行重构，因为如果出现问题，您随时可以回滚。

2）我正在改变的功能是否存在单元测试？ 如果是这样，那就太好了！ 重构。 这里的危险在于单元测试的存在并不表明所述单元测试的准确性和范围。 好的单元测试应该能够发现任何重大变化。

3）我是否彻底理解了我正在重构的代码？ 如果没有源代码控制和测试，并且我并不真正理解我正在更改的代码，那么这是一个危险信号。 在重构之前我需要更熟悉代码。

在情况 #3 中，我可能会花时间实际跟踪当前正在使用我正在重构的方法的所有代码。 根据代码的范围，这可能很容易，也可能不可能（即，如果它是公共 API）。 如果归结为公共 API，那么您确实需要从业务角度理解代码的初衷。

Answer 3

我只会在测试已经到位的情况下重构它。 如果不是，通常不值得我花时间为可能有效的代码编写测试和重构。

Answer 4

这是一个很小的反模式，但它让我非常恼火，每当我发现它时，我都会立即删除它。 在 C（或 C++ 或 Java）中

if (p)
    return true;
else
    return false;

，

return p;

在 Scheme 中

(if p #t #f)

，

p

在 ML 中

if p then true else false

，

p

我几乎完全在本科生编写的代码中看到这种反模式。 我绝对不是编造的！

Answer 5

每当我遇到它并且我不认为改变它会引起问题（例如，我可以足够理解它，我知道它的作用。例如，巫毒的水平很低）。

Answer 6

只有当我修改代码有其他原因时，我才懒得去更改它。

我愿意走多远取决于我对不会破坏任何东西的信心以及我自己对代码的更改将有多大。

Answer 7

这是展示单元测试好处的绝佳情况。

如果单元测试到位，开发人员可以勇敢而积极地重构他们可能遇到的奇怪编写的代码。 如果它通过了单元测试并且您提高了可读性，那么您已经完成了当天的善行并且可以继续前进。

如果没有单元测试，简化充满巫术的复杂代码会带来破坏代码的巨大风险，甚至不知道您引入了新的错误！ 因此，大多数开发商都会采取谨慎的路线并继续前进。

Answer 8

为了进行简单的重构，我尝试清理深层嵌套的控制结构和非常长的函数（超过一屏的文本）。 然而，在没有充分理由的情况下重构代码并不是一个好主意（尤其是在一个大的开发团队中）。 一般来说，除非重构会对代码产生很大的改进或修复一个严重的错误，否则我会尽量不去管它。

不是按说重构，而是作为一般内务处理的问题，当我开始处理模块时，我通常会做这些事情：

• 删除愚蠢的评论
  • 仅说明函数签名已经说明的内容的注释
  • 纯粹白痴的评论，例如“就像它看起来的样子”
  • 文件顶部的更改日志（我们有版本控制是有原因的）
  • 任何明显与代码不同步的 API 文档
• 删除注释掉的代码块
• 添加版本控制标记，例如 $Id$（如果丢失）
• 修复空白问题（但这可能会让其他人烦恼，因为您的名字会出现在一个文件中的很多行中） diff 即使您所做的只是更改空格）
  • 删除行尾的空格
  • 更改制表符 -> 空格（因为这是我们工作的惯例）

Answer 9

超过（可以说）三行或四行重复代码总是让我考虑重构。 另外，我倾向于经常移动代码，将我预计会更频繁使用的代码提取到一个单独的位置 - 一个具有自己明确定义的目的和职责的类，或者静态类的静态方法（通常放置在我的 Utils.* 命名空间）。

但是，要回答你的问题，是的，在很多情况下，缩短代码并不一定意味着使其结构良好且可读。 在 C# 中使用 ?? 运算符是另一个示例。 您还必须考虑的是您选择的语言中的新功能 - 例如 LINQ 可以用来以非常优雅的方式做一些事情，但也可以使非常简单的事情变得非常难以阅读并且过度复杂的。 您需要非常仔细地权衡这两件事，最终一切都归结为您的个人品味，尤其是经验。

好吧，这是另一个“视情况而定”的答案，但恐怕必须如此。

Answer 10

如果重构使代码更容易阅读，那么对我来说最常见的是重复代码，例如仅第一个/最后一个命令不同的 if/else。

if($something) {
  load_data($something);
} else {
  load_data($something);
  echo "Loaded";
  do_something_else();
}

Answer 11

我几乎总是将 >2 个相似的条件句分解为一个开关……最常见的是关于枚举。 我将简短地返回而不是冗长的声明。

ex:

if (condition) {
  //lots of code
  //returns value
} else {
  return null;
}

变成:

if (!condition)
  return null;

//lots of code..
//return value

提前突破减少了额外的缩进，并减少了长代码...另外作为一般规则，我不喜欢具有超过 10-15 行代码的方法。 我喜欢具有单一目的的方法，即使在内部创建更多私有方法也是如此。

Answer 12

通常，如果我只是浏览代码而不是积极处理代码，我就不会重构代码。

但有时 ReSharper 会指出一些我无法抗拒 Alt+Enter 的东西。 :)

Answer 13

如果我了解块的输入和输出集，我倾向于重构很长的函数和方法。

这对可读性有无止境的帮助。

Answer 14

在完成以下步骤后，我只会重构我遇到的并且没有积极处理的代码：

• 与代码的作者交谈（并不总是可能）以弄清楚该代码的作用。 即使对于我来说这段代码在做什么是显而易见的，它总是有助于理解为什么作者可能决定以某种方式做某事背后的基本原理。 花几分钟讨论它不仅可以帮助原作者理解您的观点，还可以在团队内部建立信任。
• 了解或找出该代码正在做什么，以便在重构后对其进行测试（带有单元测试的构建过程在这里非常有帮助。它使整个事情变得快速而简单）。 在更改之前和之后运行单元测试，并确保您的更改不会造成任何破坏。
• 向团队发送提醒（如果与其他人合作），让他们知道即将发生的变化，这样当变化实际发生时，没有人会感到惊讶

Answer 15

为了重构而重构是万恶之源之一。 请永远不要这样做。 （单元测试确实在一定程度上缓解了这个问题）。

Answer 16

大量注释掉的代码是反模式吗？ 虽然不是具体的代码，（它是注释）我看到很多这样的行为，这些人不了解如何使用源代码控制，并且希望保留旧代码以供以后使用，以便他们可以更轻松地返回，如果引入了一个错误。 每当我看到大量被注释掉的代码时，我几乎总是将它们全部删除。

Answer 17

我尝试根据以下因素进行重构：

1. 我是否足够了解发生了什么？
2. 如果此更改破坏了代码，我可以轻松恢复吗？
3. 如果更改破坏了构建，我是否有足够的时间来恢复更改。

有时，如果我有足够的时间，我会重构来学习。 例如，我知道我的更改可能会破坏代码，但我不知道在哪里以及如何破坏。 所以我无论如何都会改变它，以找出它的问题所在以及原因。 这样我就可以更多地了解代码。

我的领域是移动软件（手机），其中大部分代码驻留在我的电脑上，不会影响其他人。 由于我还为公司维护 CI 构建系统，因此我可以在重构的代码上运行完整的产品构建（适用于所有手机），以确保它不会破坏任何其他内容。 但这是我个人的奢侈，你可能没有。

Answer 18

如果全局常量和枚举被认为重构风险较低，我倾向于对其进行大量重构。 也许只是这样，但像 ClientAccountStatus 枚举这样的东西应该位于或接近 ClientAccount 类，而不是位于 GlobalConstAndEnum 类中。