7

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

您什么时候会触摸与您正在处理的代码不直接相关的代码以使其更清晰?

4

18 回答 18

16

我曾经在重构并遇到过类似这样的代码:

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

Resharper 指出 .ToString() 是多余的,所以我把它拿出来了。不幸的是,这最终破坏了代码。每当 MySessionVar 为空时,它都不会导致代码依赖的 NullReferenceException 将其撞到 catch 块。我知道,这是一些可悲的代码。但我确实从中吸取了很好的教训。不要依靠工具来快速浏览旧代码来帮助您进行重构——请自己思考。

我最终将其重构如下:

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

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

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

1) 系统是否处于源代码控制之下?如果是这样,请继续重构,因为如果出现问题,您总是可以回滚。

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

3) 我是否完全理解我正在重构的代码?如果没有源代码控制和测试,而且我并不真正理解我正在更改的代码,那就是一个危险信号。在重构之前,我需要对代码更加熟悉。

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

于 2008-12-24T00:34:57.487 回答
7

如果测试已经到位,我只重构它。如果没有,通常不值得我花时间为可能工作的代码编写测试重构。

于 2008-12-23T23:18:19.967 回答
6

这是一个很小的、次要的反模式,但它让我很恼火,以至于每当我发现它时,我都会立即删除它。在 C(或 C++ 或 Java)中

if (p)
    return true;
else
    return false;

变成

return p;

在方案中,

(if p #t #f)

变成

p

在机器学习中

if p then true else false

变成

p

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

于 2008-12-24T01:30:50.917 回答
5

每当我遇到它并且我认为更改它不会导致问题时(例如,我可以充分理解它,以至于我知道它的作用。例如,巫毒教的水平很低)。

于 2008-12-23T22:49:23.940 回答
3

如果有其他原因我正在修改代码,我只会费心去改变它。

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

于 2008-12-23T23:09:32.543 回答
3

这是展示单元测试好处的好时机。

如果单元测试到位,开发人员可以勇敢而积极地重构他们可能遇到的奇怪编写的代码。如果它通过了单元测试并且你增加了可读性,那么你已经做了一天的好事,可以继续前进。

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

于 2008-12-23T23:32:39.767 回答
3

对于简单的重构,我尝试清理深度嵌套的控制结构和非常长的函数(超过一屏的文本)。然而,在没有充分理由的情况下重构代码并不是一个好主意(尤其是在一大群开发人员中)。一般来说,除非重构会对代码有很大的改进或修复一个严重的罪过,否则我会尽量不去管它。

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

  • 删除愚蠢的评论
    • 仅说明函数签名已经说明的注释
    • 纯粹白痴的评论,例如“看起来就是这样”
    • 文件顶部的变更日志(我们有版本控制是有原因的)
    • 任何与代码明显不同步的 API 文档
  • 删除注释掉的代码块
  • 如果缺少版本控制标签,请添加 $Id$
  • 修复空格问题(这可能会令其他人烦恼,因为即使您所做的只是更改空格,您的名字也会出现在 diff 中的很多行中)
    • 删除行尾的空格
    • 更改制表符-> 空格(因为这是我工作的约定)
于 2008-12-24T04:43:46.360 回答
1

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

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

于 2008-12-23T22:50:43.733 回答
1

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

if($something) {
  load_data($something);
} else {
  load_data($something);
  echo "Loaded";
  do_something_else();
}
于 2008-12-23T22:52:51.530 回答
1

超过(可以说)三四行重复代码总是让我想到重构。此外,我倾向于大量移动代码,将我预测会更频繁使用的代码提取到一个单独的地方——一个具有自己明确定义的目的和职责的类,或者一个静态类的静态方法(通常放在我的 Utils.* 命名空间)。

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

好吧,这是另一个“取决于”的答案,但恐怕必须如此。

于 2008-12-24T00:48:32.843 回答
1

我几乎总是将> 2个类似的条件分解成一个开关......最常见的是关于枚举。我会做一个简短的回报而不是一个长的声明。

前任:

如果(条件){
  //很多代码
  //返回值
} 别的 {
  返回空值;
}

变成:

如果(!条件)
  返回空值;

//很多代码..
//返回值

提前突破可以减少额外的缩进,并减少长代码......通常我不喜欢超过 10-15 行代码的方法。我喜欢方法有一个单一的目的,即使在内部创建更多的私有方法。

于 2008-12-24T03:12:00.417 回答
0

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

这有助于无止境的可读性。

于 2008-12-23T23:11:44.970 回答
0

在完成以下步骤后,我只会重构我遇到并且没有积极工作的代码:

  • 与代码的作者交谈(并非总是可能)以弄清楚那段代码的作用。即使我很清楚这段代码在做什么,它总是有助于理解为什么作者可能决定以某种方式做某事的理由。花几分钟谈论它不仅可以帮助原作者理解您的观点,还可以在团队内部建立信任。
  • 了解或找出那段代码在做什么,以便在重构后对其进行测试(带有单元测试的构建过程在这里非常有用。它使整个事情变得快速而简单)。在更改之前和之后运行单元测试,并确保没有任何内容因您的更改而中断。
  • 向团队发出提示(如果与其他人一起工作)并让他们知道即将发生的变化,这样当变化实际发生时没有人会感到惊讶
于 2008-12-23T23:32:23.917 回答
0

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

于 2008-12-24T01:23:12.320 回答
0

大量注释掉的代码是反模式吗?虽然没有具体编码,(它的评论)我看到很多这种行为与不了解如何使用源代码控制的人,以及想要保留旧代码以供以后使用,以便他们可以更轻松地返回引入了一个错误。每当我看到大量被注释掉的代码时,我几乎总是将它们全部删除。

于 2008-12-24T03:19:05.753 回答
0

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

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

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

我的领域是移动软件(手机),其中大部分代码都驻留在我的 PC 上,不会影响其他人。由于我还为我的公司维护 CI 构建系统,我可以在重构代码上运行完整的产品构建(适用于所有手机),以确保它不会破坏其他任何东西。但那是我个人的奢侈品,你可能没有。

于 2008-12-24T03:26:11.853 回答
0

如果可以将全局常量和枚举视为低重构风险,我倾向于对其进行相当多的重构。也许只是这样,但是像 ClientAccountStatus 枚举之类的东西应该在 ClientAccount 类中或附近,而不是在 GlobalConstAndEnum 类中。

于 2008-12-24T05:46:06.687 回答
0

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

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

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

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

于 2009-05-21T17:07:00.833 回答