注册 登录  
 加关注
   显示下一条  |  关闭
温馨提示!由于新浪微博认证机制调整,您的新浪微博帐号绑定已过期,请重新绑定!立即重新绑定新浪微博》  |  关闭

BCB-DG's Blog

...

 
 
 

日志

 
 

FieldByName, FindField: too convenient to be honest  

2010-12-01 08:25:27|  分类: Delphi |  标签: |举报 |字号 订阅

  下载LOFTER 我的照片书  |

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):

  while not ADataSet.Eof do
begin
[...]
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;
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;

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;
while not AdoQuery1.Eof do
begin
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 added
begin
Assert(AdoQuery1.Active, 'Dataset is not active!');
try
AdoQuery1.DisableControls;
AField := AdoQuery1.FieldByName('MyFieldName'); // <= line added
AdoQuery1.First;
while not AdoQuery1.Eof do
begin
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?

  评论这张
 
阅读(1046)| 评论(0)
推荐 转载

历史上的今天

评论

<#--最新日志,群博日志--> <#--推荐日志--> <#--引用记录--> <#--博主推荐--> <#--随机阅读--> <#--首页推荐--> <#--历史上的今天--> <#--被推荐日志--> <#--上一篇,下一篇--> <#-- 热度 --> <#-- 网易新闻广告 --> <#--右边模块结构--> <#--评论模块结构--> <#--引用模块结构--> <#--博主发起的投票-->
 
 
 
 
 
 
 
 
 
 
 
 
 
 

页脚

网易公司版权所有 ©1997-2017