FieldByName, FindField确实太好用了

Google翻译 整理

我可能不认识你，但我都不知道有多少次看到这样的代码模式了：

while not ADataSet.Eof do
 begin
   [...]
   ADataSet.FieldByName('MyFieldName1').SomePropertyOrMethod;
   ADataSet.FieldByName('MyFieldName2').SomePropertyOrMethod;
   [...]
   ADataSet.Next;
 end;

这种情况用FieldByName肯定是非常方便，有很多好处：

   *清晰度和可读性。代码的意图是明显的，数据正是你需要的,这是毫无疑问。 
   *接近。你创造了你您需要的字段的引用。 
   *灵活性。你不会被设计时字段阻碍了，你的DataSet可以代表多个查询，你只需要知道内容中有一个MyFieldName的列即可。 
   *安全。 FieldByName永远不会返回nil，如果没有确实实地字段存在,因为它会引发一个异常。所以，你直接使用属性或方法时，确信不会获得一个av(地址访问错误）。

但是，有一个重大的问题：

   *这是搜索，每一个DataSet的记录都要查询已经找到的同一字段。这样的代码有老年痴呆症。 
   *他们知道每调用FieldByName主要是调用FindField，会在主循环引进3个新子循环。象上面的两个字段，就会引入6个子循环，而且是每个数据行都这样。

如果这只是一个例子或者一些演示代码,这样似乎并不认为不好，但一旦它发布后，有人会抓住一个片段和一些拷贝粘贴后，他就会出现在产品代码中。

事实上，我可以告诉你，这（无数）公司的产品代码实际上包含了如下（匿名，但真实的）代码数十例：

 aQuery.ParamByName('Zone_mask').DataType := ftLargeint;
 aQuery.ParamByName('Zone_mask').Value    := aZoneMask;
 aQuery.ParamByName('Start_date').AsDateTime := fStartDate;
 aQuery.ParamByName('End_date').AsDateTime   := fEndDate;

 aQuery.Open;
 while not aQuery.EOF do
 begin
   effectiveDeviceInfo :=
     TEffectiveDeviceInfo.Create(
        aQuery.FieldByName('Device_code_name').Value,
        aQuery.FieldByName('Device_descr').Value,
        aQuery.FieldByName('Toggle_times').Value,
        aQuery.FieldByName('Comments').Value,
        aQuery.FieldByName('Effective_start_date').Value,
        aQuery.FieldByName('Effective_end_date').Value);

   fChannelDevice.AddObject(
     self.MakeSortKey(
       aQuery.FieldByName('Device_code_id').Value,
       aQuery.FieldByName('Channel_int').Value),
     effectiveDeviceInfo);

   aQuery.Next;
 end;
 aQuery.Close;

数据集的每个记录将增加在8至24个循环,而数据集可能有成千上万的记录
诚然，如果字段数是合理的这将是小循环，但仍然...

所以，当我看到有人发布的代码片段，看起来很不错（包括verify Active, DisableControls, try..finally等），一看到循环内使用FieldByName,就很泄气：

begin
 Assert(AdoQuery1.Active, 'Dataset is not active!');
 try
   AdoQuery1.DisableControls;
   AdoQuery1.First;
   while not AdoQuery1.Eof do
   begin
     AdoQuery1.Edit;
     AdoQuery1.FieldByName('MyFieldName').Value := Edit1.Text;
     AdoQuery1.Post;
     AdoQuery1.Next;
   end;
 finally
   AdoQuery1.EnableControls;
 end;
end;

一个小小的改进，在循环之前创建一个局部变量。它增加了2行，但代码是好多了：

var
 AField : TField;  // <= 增加的行
begin
 Assert(AdoQuery1.Active, 'Dataset is not active!');
 try
   AdoQuery1.DisableControls;
   AField := AdoQuery1.FieldByName('MyFieldName'); // <= 增加的行
   AdoQuery1.First;
   while not AdoQuery1.Eof do
   begin
     AdoQuery1.Edit;
     AField.Value := Edit1.Text; // <= 这行修改了
     AdoQuery1.Post;
     AdoQuery1.Next;
   end;
 finally
   AdoQuery1.EnableControls;
 end;
end;

如果说，因为记录的数量太小，在DataSet循环外处理FieldByName收益不多，而不要去“过早优化”,

我很好奇，想知道如果你在你的代码库中找到一个循环FieldByName或FindField grep的时候，你会得到什么... 
你还有什么？

 

英文原文

I don’t know about you, but I can’t count anymore the number of times I’ve seen this code pattern (in code snippets online as well as in production code):

whilenot ADataSet.Eof dobegin[...] ADataSet.FieldByName('MyFieldName1').SomePropertyOrMethod; ADataSet.FieldByName('MyFieldName2').SomePropertyOrMethod;[...] ADataSet.Next;end;

Using FieldByName in a case like this is certainly very convenient and has a lot of advantages:

• Clarity and readability. The intent of the code is obvious and there is no question about which Data point you work with.

• Proximity. You create your Field reference exactly where you need it.

• Flexibility. You’re not stuck with design-time Fields, your DataSet can represent multiple queries, you just need to know that you have a ‘MyFieldName’ column in this context.

• Security. FieldByName never returns nil because it raises an exception if the Field does no exist. So, you’re sure not to get an AV when directly using a property or method.

But is has a major problem:

• It is searching -again- for the same already found Field at each and every record of the DataSet. Like this code has Alzheimer.

• Knowing that each call to FieldByName is mainly a call to FindField, that can introduce up to 3 new sub-loops within your main loop. With our 2-Field example above, that’s 6 sub-loops that can be added for every row.

It does not seem that bad if it is just an example or some demo code, but as soon as it is published, someone will grab a snippet and a few copy’n'haste later it ends up in production code.

And indeed, I can tell you that this (untold) company’s production code actually contained dozens of cases like the following (anonymized but real) code:

aQuery.ParamByName('Zone_mask').DataType := ftLargeint; aQuery.ParamByName('Zone_mask').Value := aZoneMask; aQuery.ParamByName('Start_date').AsDateTime := fStartDate; aQuery.ParamByName('End_date').AsDateTime := fEndDate; aQuery.Open;whilenot aQuery.EOF dobegin effectiveDeviceInfo := TEffectiveDeviceInfo.Create( aQuery.FieldByName('Device_code_name').Value, aQuery.FieldByName('Device_descr').Value, aQuery.FieldByName('Toggle_times').Value, aQuery.FieldByName('Comments').Value, aQuery.FieldByName('Effective_start_date').Value, aQuery.FieldByName('Effective_end_date').Value); fChannelDevice.AddObject( self.MakeSortKey( aQuery.FieldByName('Device_code_id').Value, aQuery.FieldByName('Channel_int').Value), effectiveDeviceInfo); aQuery.Next;end; aQuery.Close;

That’s between 8 and 24 added loops for each record in this dataset which could have tens of thousands of records!
Granted, that would be small loops if the number of fields is reasonable, but still…

So, I cringe when I see someone posting on StackOverflow a code snippet that looks good (verify Active, DisableControls, try..finally) but has the FieldByName inside the loop:

begin Assert(AdoQuery1.Active,'Dataset is not active!');try AdoQuery1.DisableControls; AdoQuery1.First;whilenot AdoQuery1.Eof dobegin AdoQuery1.Edit; AdoQuery1.FieldByName('MyFieldName').Value := Edit1.Text; AdoQuery1.Post; AdoQuery1.Next;end;finally AdoQuery1.EnableControls;end;end;

And kudos to the poster who changed it after just a little nudge to create a local Field variable before the loop. It adds 2 more lines per Field, but the code is much better:

var AField : TField;// <= line addedbegin Assert(AdoQuery1.Active,'Dataset is not active!');try AdoQuery1.DisableControls; AField := AdoQuery1.FieldByName('MyFieldName');// <= line added AdoQuery1.First;whilenot AdoQuery1.Eof dobegin AdoQuery1.Edit; AField.Value := Edit1.Text; AdoQuery1.Post; AdoQuery1.Next;end;finally AdoQuery1.EnableControls;end;end;

Now, don’t go “premature optimization” on me, as the cases where the number of records is too small to benefit from ousting the FieldByName from the DataSet loop are pretty rare.

I would be curious to know what you get if you do a grep in your code base to find FieldByName or FindField in loops…
Do you have any?