问题 参考滥用:值得清理?


我继承了一些使用的代码 REF 关键字广泛且不必要。原始开发人员显然担心对象会被克隆为原始类型,如果 REF 没有使用,并且在编写50k +代码行之前没有费心去研究这个问题。

这与其他不良编码实践相结合,已经创造了一些表面上非常危险的情况。例如:


Customer person = NextInLine(); 
//person is Alice
person.DataBackend.ChangeAddress(ref person, newAddress);
//person could now be Bob, Eve, or null

你能想象走进商店改变你的地址,走出一个完全不同的人吗?


可怕,但是 在实践中 指某东西的用途 REF 在这个应用程序中似乎无害多余。我无法证明清理它需要花费大量时间。为了帮助推销这个想法,我提出了以下问题:

如何不必要地使用ref是破坏性的?

我特别关心维护。具有示例的合理答案是优选的。

我们也欢迎你争辩说没有必要进行清理。


7027
2018-04-14 18:51


起源



答案:


我想说最大的危险是如果参数设置为 null 由于某种原因在函数内部:

public void MakeNull(ref Customer person)
{
    // random code
    person = null;
    return;
}

现在,你不仅仅是一个 不同 那个人,你已经完全被删除了!

只要开发此应用程序的人都理解:

默认情况下,对象引用按值传递。

和:

随着 ref 关键字,对象引用通过引用传递。

如果代码现在按预期工作,并且您的开发人员了解其中的差异,则可能不值得将其全部删除。


8
2018-04-14 18:56





我将添加我见过的ref关键字的最坏用法,该方法看起来像这样:

public bool DoAction(ref Exception exception) {...}

是的,您必须声明并传递一个Exception引用才能调用该方法,然后检查该方法的返回值,以查看是否已捕获并通过ref异常返回异常。


4
2018-04-14 18:59



老实说,这实际上是ref关键字的有趣用法。 out关键字可能更合适,因此您实际上不必初始化异常,然后在方法之后检查ex == null否则抛出ex。我更喜欢从具有属性Success / Valid等的服务函数以及信息/警告/错误消息的集合返回基于IList的消息类,但我认为他们的解决方案同样有效。 - Chris Marisic
我认为代码显示了对异常处理应该如何工作的基本误解,至少可以这么说。如果要返回错误列表,可以在每次捕获异常时设置内部异常,这样就可以得到一堆好的从高到低的消息,或者创建一个自定义异常,并将警告列表作为属性。我唯一能想到你实际上像数据对象一样传递异常的时候是某种日志记录或解析器类。 - Paul


答案:


我想说最大的危险是如果参数设置为 null 由于某种原因在函数内部:

public void MakeNull(ref Customer person)
{
    // random code
    person = null;
    return;
}

现在,你不仅仅是一个 不同 那个人,你已经完全被删除了!

只要开发此应用程序的人都理解:

默认情况下,对象引用按值传递。

和:

随着 ref 关键字,对象引用通过引用传递。

如果代码现在按预期工作,并且您的开发人员了解其中的差异,则可能不值得将其全部删除。


8
2018-04-14 18:56





我将添加我见过的ref关键字的最坏用法,该方法看起来像这样:

public bool DoAction(ref Exception exception) {...}

是的,您必须声明并传递一个Exception引用才能调用该方法,然后检查该方法的返回值,以查看是否已捕获并通过ref异常返回异常。


4
2018-04-14 18:59



老实说,这实际上是ref关键字的有趣用法。 out关键字可能更合适,因此您实际上不必初始化异常,然后在方法之后检查ex == null否则抛出ex。我更喜欢从具有属性Success / Valid等的服务函数以及信息/警告/错误消息的集合返回基于IList的消息类,但我认为他们的解决方案同样有效。 - Chris Marisic
我认为代码显示了对异常处理应该如何工作的基本误解,至少可以这么说。如果要返回错误列表,可以在每次捕获异常时设置内部异常,这样就可以得到一堆好的从高到低的消息,或者创建一个自定义异常,并将警告列表作为属性。我唯一能想到你实际上像数据对象一样传递异常的时候是某种日志记录或解析器类。 - Paul


你能弄清楚为什么代码的发起者认为他们需要将参数作为参考吗?是因为他们确实更新了它然后删除了功能,还是因为他们当时不理解c#?

如果你认为清理是值得的,那么继续吧 - 特别是如果你现在有时间的话。当出现真正的问题时,您可能无法正确地修复它,因为它很可能是一个紧急的错误修复,您将没有时间做正确的工作。


2
2018-04-14 18:59



清理的一个重要部分是确定哪些(如果有的话)ref实际使用得当。我看到的90%的事件显然不包含对引用的任何更改,并且从未这样做。 - Karmic Coder


在C#中修改方法中参数的值是很常见的,因为它们通常是按值而不是由ref。这适用于参考和价值类型;例如,将引用设置为null将更改原始引用。当其他开发人员像往常一样工作时,这可能会导致非常奇怪和痛苦的错误。使用ref参数创建递归方法是不行的。

除此之外,您对ref可以传递的内容有限制。例如,您不能传递常量值,只读字段,属性等,因此在使用ref参数调用方法时需要许多辅助变量。

最后但并非最不重要的是性能如果可能不太好,因为它需要更多间接(ref只是一个需要在每次访问时解决的参考)并且还可以使对象保持更长时间,因为引用不会出现范围很快。


1
2018-04-14 19:00





对我来说,闻起来就像一个C ++开发人员做出了无根据的假设。

我会谨慎地改变那些有效的东西。 (我假设它有效,因为你没有评论它被破坏,只是它很危险)。

你要做的最后一件事是打破一些微妙的东西,并且必须花一周的时间来追踪问题。

我建议你随时清理 - 一次一种方法。

  1. 找到一个使用ref的方法,确定它不是必需的。
  2. 更改方法签名并修复调用。
  3. 测试。
  4. 重复。

虽然您遇到的具体问题可能比大多数情况更严重,但您的情况非常普遍 - 拥有大量代码库,不符合我们目前对“正确的方法“做事。

批发“升级”经常遇到困难。随你进行重构 - 在处理它们时提高规格 - 更加安全。

这是建筑行业的先例。例如,除非出现问题,否则不需要触摸较旧(例如,19世纪)建筑物中的电线。当有  但问题是,新工作必须按现代标准完成。


1
2018-04-14 19:42



这是一个很好的比喻...... - John Rasch


我会尝试解决它。只需使用正则表达式执行解决方案宽字符串替换,然后检查单元测试。我知道这可能会破坏代码。但是你多久使用一次参考?几乎从不,对吧?鉴于开发人员不知道它是如何工作的,我认为它在某个地方(它应该的方式)使用的可能性更小。如果代码中断 - 好吧,回滚......


0
2018-04-14 19:30





如何不必要地使用ref是破坏性的?

其他答案涉及语义问题,这绝对是最重要的事情。好的代码是自我记录的,当我给出ref参数时,我会假设它  更改。如果没有,则API无法自我记录。

但为了好玩,我们如何看待另一个方面 - 性能?

void ChangeAddress(ref Customer person, Address address)
{
    person.Address = address;
}

这里 person 是对引用的引用,因此每当您访问它时都会引入一些间接引用。让我们看看这可能转化为的一些组件:

mov eax, [person]           ; load the reference to person.
mov [eax+Address], address  ; assign address to person.Address.

现在非ref版本:

void ChangeAddress(Customer person, Address address)
{
    person.Address = address;
}

这里没有间接,所以我们可以摆脱一个阅读:

mov [person+Address], address  ; assign address to person.Address.

在实践中,人们希望.NET缓存 [person] 在里面 ref 版本,通过多次访问分摊间接成本。实际上,除了像这里的琐碎方法之外,指令数量实际上不会下降50%。


0
2017-09-04 23:58