1

考虑以下

constructor TSettlement.Assign( const OldInst : TSettlement; const ResetFsToo: Boolean );
begin
  //inherited
  Create;
  if OldInst = nil then
    exit;

  Self.Acceptancedate := OldInst.Acceptancedate;
  // etc etc
end;

并请考虑代码中其他地方的这些调用

SettInst.Assign(DisplaySett, False);

DisplaySett := TSettlement.Assign(nil, False);

NewInst := TSettlement.Assign( Displaysett, False );

和（也许是最糟糕的）

  if OldList.Count > 0 then
    for loop := 0 to OldList.Count -1 do  
      Self.Add(TSettlement.Assign(OldList.Data[loop], True));

这是泄漏代码，我反对将方法名称“分配”用于构造函数，原因我认为很明显，但我没有义务修复它。

我想改进它，因为我对自己的工作感到自豪。

我建议将Assign方法从构造函数更改为过程，并删除对Create(). 这将需要我在应用程序的许多地方更改代码。显然，这样做是有风险的。

在我深入研究并开始破解之前，任何人都可以建议我应该考虑的任何替代方法吗？

有没有我可能没有想到的陷阱我应该注意？

4

1 回答 1

4

该构造函数不一定泄漏。代码确实有效是合理的。然而，它是不可穿透的，并导致调用代码的语义难以从外部辨别。你应该重构。

有一种简单的重构方法。关键是您将两种操作模式分解为单独的功能：

  1. 一个名为 Create 的构造函数，其作用类似于普通的 Delphi 构造函数。你似乎已经有了这个。
  2. 将一个实例的内容复制到另一个实例的过程。这很可能被命名为Assign。

所以，这是竞选计划：

  1. 将 Assign 更改为过程而不是构造函数。
  2. 处理可以在当前分配中找到的对 Create 的调用。您需要从Assign 中删除它，但要确保它所做的一切在Assign 运行时仍然发生。
  3. 现在所有对 Assign 的构造函数模式调用都无法编译。所以我们修复它们。
  4. 通过 nil 的，转换为 Create 的构造函数调用。
  5. 其他的需要调用 Create 构造函数，然后调用 Assign。

您可能需要一个重载的 Create 来接收现有实例并调用 Assign。那可能很方便。

这将使您能够完成您今天所做的所有事情，但会避免在实例上调用构造函数，这总是一个坏主意。

于 2013-04-16T23:06:36.870 回答