delphi - 有关重构一些不良代码的建议

Question

考虑以下

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”用于构造函数，但我没有义务对其进行修复。

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

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

在深入学习之前，有人可以建议我考虑其他替代方法吗？

我可能没有想到我应该意识到的潜在陷阱吗？

Answer 1

该构造函数不一定泄漏。该代码实际可行，这是合理的。但是，它是不可渗透的，并导致调用其语义难以从外部识别的代码。您应该重构。

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


名为Create的构造函数的行为类似于普通的Delphi构造函数。您似乎已经拥有了这个。
将一个实例的内容复制到另一个实例的过程。可以将其命名为Assign。


所以，这是竞选计划：


将Assign更改为过程而不是构造函数。
处理可以在当前“分配”中找到的“创建”调用。您需要将其从Assign中删除，但是请确保在运行Assign时，它所做的一切仍然会发生。
现在，所有对Assign的构造函数模式调用均无法编译。因此，我们对其进行了修复。
传递nil的变量转换为Create的构造函数调用。
其他的需要调用Create构造函数，然后调用Assign。


您可能希望接收现有实例并调用Assign的重载Create。那可能很方便。

这样一来，您就可以完成今天的所有工作，但可以避免在实例上调用构造函数，这总是一个坏主意。