Delphi: you should avoid the `with` statement as it makes your code less future proof
Posted by jpluimers on 2013/03/27
As I wrote before, I’m with the [WayBack] Delphi with haters camp, and this is why:
Using the [WayBack] with statement in Delphi makes your code less future proof.
Originally, the
with
statement in Pascal was argumented in part of allowing compiler optimisations:PASCAL User Manual and Report – Kathleen Jensen, Niklaus Wirth – Google Books
The with clause effectively opens the scope containing field identifiers of the specified record variable, so that the field identifiers may occur as variable identifiers. (Thereby providing an opportunity for the compiler to optimize the qualified statement.)
Screenshots of this 1975 book are below the fold.
The Delphi (actually even before that Turbo Pascal compiler) has no measurable difference between
with
and non-with
code.The debugger however, still does not support with, and there are other drawbacks of which one is below.
The below code example is just one of many. I show it because I recently bumped into doing some long overdue code porting to Delphi XE3.
Since I’ve been bitten by using with
a couple of times before, it didn’t take me long to find the cause.
Example code where FIConData
is of type NOTIFYICONDATAW
that used to compile fine:
with FIconData do begin cbSize := SizeOf(FIconData); Wnd := Self.Handle; uID := $DEDB; uFlags := NIF_MESSAGE or NIF_ICON or NIF_TIP; hIcon := Application.Icon.Handle; uCallbackMessage := WM_CAS400NTIcon; StrCopy(szTip, PChar(Caption)); end;
Well, as of Compiler Version 20, it doesn’t compile any more.
The reason is that the _NOTIFYICONDATAW got changed, because the underlying Windows API NOTIFYICONDATA structure changed to accommodate new Windows Vista features.
But because of backward compatibiliy, you cannot pass the full new structure when you are running on Windows versions prior to Windows Vista.
Hence this piece of code to calculate the right structure size in the ShellApi
unit as of Delphi 2009:
class function _NOTIFYICONDATAW.SizeOf: Integer; static; begin if Win32MajorVersion >= 6 then // Size of complete structure Result := System.SizeOf(_NOTIFYICONDATAW) else // Platforms prior to Vista do not recognize the fields guidItem and hBalloonIcon Result := System.SizeOf(_NOTIFYICONDATAW) - System.SizeOf(TGUID) - System.SizeOf(Winapi.Windows.HICON); end;
SizeOf
is a perfectly good name for this, but it clashes when you use the with
statement.
Three other SizeOf
functions got added. This is the complete list of four:
- Unit Windows:
- class function tagNONCLIENTMETRICSA.SizeOf: Integer;
- class function tagNONCLIENTMETRICSW.SizeOf: Integer;
- Unit CommCtrl:
- class function MCHITTESTINFO.SizeOf: Integer;
- Unit ShellApi:
- class function _NOTIFYICONDATAW.SizeOf: Integer; static;
You are tempted to rewrite the with
like this:
with FIconData do begin {$if CompilerVersion >= 20} cbSize := SizeOf(); {$else} cbSize := SizeOf(FIconData); {$ifend CompilerVersion >= 20} Wnd := Self.Handle; uID := $DEDB; uFlags := NIF_MESSAGE or NIF_ICON or NIF_TIP; hIcon := Application.Icon.Handle; uCallbackMessage := WM_CAS400NTIcon; StrCopy(szTip, PChar(Caption)); end;
But you should in fact rewrite it like this, so you are sure from which entity each value comes:
{$if CompilerVersion >= 20} FIconData.cbSize := FIconData.SizeOf(); {$else} FIconData.cbSize := SizeOf(FIconData); {$ifend CompilerVersion >= 20} FIconData.Wnd := Self.Handle; FIconData.uID := $DEDB; FIconData.uFlags := NIF_MESSAGE or NIF_ICON or NIF_TIP; FIconData.hIcon := Application.Icon.Handle; FIconData.uCallbackMessage := WM_CAS400NTIcon; StrCopy(FIconData.szTip, PChar(Caption));
–jeroen
Delphi `with` post and discussion revisited (via: wiert.me and LinkedIn) « The Wiert Corner – irregular stream of stuff said
[…] A bit more than a year ago, I wrote about Delphi: you should avoid the `with` statement as it makes your code less future proof. […]
Delphi: `with` dos and don’ts (more of the latter though). « The Wiert Corner – irregular stream of stuff said
[…] a year ago, I wrote about Delphi: you should avoid the `with` statement as it makes your code less future proof. Then I already tweeted I would follow up. Time to do it now […]
Bruce McGee said
In that other thread, I also posted this quote:
“Any fool can write code that a computer can understand. Good programmers write code that humans can understand.”
And suggested that I probably shouldn’t discourage the use of With because I’ve made a not bad living rehabilitating projects that were littered with them.
I remember a discussion with another developer (my employer at the time) where he defended having With statements nested three deep. In his mind, he was being pragmatic. In my mind, I was the one who had to maintain and extend that code in production. It got changed.
jpluimers said
Thanks for the response. These are really relevant reasons for not using `with`.
Bruce McGee said
Reposted by request from LinkedIn thread pointing to this blog post:
The use of With is grounds for immediate rejection in a code review.
Just based on the (hard learned) potential for conflicts that may test properly initially and then break (sometimes subtly) when you upgrade the a newer version of Delphi, update third party code, refactor your own code, or even change the order of units in a uses clause, changing scope.
Fragile code is takes more effort to maintain.
On top of this, it obfuscates context, which may make code lines shorter, but makes the intent less obvious to the reader. And if it’s bad with a single With statement, try it with more than one that are overlapped. An explaining variable is a much better solution, here.
Code that isn’t obvious in its intent is not well written.
As the saying goes: “Do not write so that you can be understood, write so that you cannot be misunderstood.”
Or, in more current terms: “Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.”
It also interferes with debugging, preventing you from inspecting values in code.
Follow Jeoren’s links for more suggestions where it is a bad practice.
Olaf Monien said
I totally agree with Jeroen! “with” has been a permanent trouble source in customer projects we have been working on. There might be “simple” cases where “with” wouldn’t hurt, but in these “simple” cases you simply don’t need “with”. If EMBT really drops “with” (like someone mentioned above), then I would be ok with that. We actually have a De-Withard almost ready ;)
Noisrev said
This is my ‘source’ where I got that info about ‘with’ drop from NextGen compiler ;)
From http://opendec.wordpress.com/2009/09/23/indy-svn-access/ you can get access to public svn of Indy project:
And see this revision:
Revision: 4823
Author: Indy-RemyLebeau
Date: 6 oct 2012 05:01:58
Message:
Checking in initial changes for NextGen compiler
After you see changes in this revision (/branches/DelphiNextGen/…), you notice that ‘with’ has been dropped.
antonio said
Many languages support local dereferencers. “with” should be fixed, not dropped. Lots of suggestions in the Bug Reporting tool from many people. But the linker must be changed to keep the debug info, thus making it delphi-specific, not generic. For this reason it’s easier to drop “with”.
Noisrev said
For now I see only 3-ways to ‘fix’ mess with ‘with’:
1) warn by compiler when it see ambiguous ‘with’ usage. Generally it should warn in *all cases* when it see ambiguous name usage. I would not mind if it complier would emit ‘error’ in this case.
[ IMO simple, clean, powerful, bug-free option ]
2) drop support for it – easy way (already done). This one also ‘fixes’ Debugging/Code completion/syntax/hint issues.
3) extend syntax to something like (inline variables)
with var a := My.Very.Long.ClassAccess, b := Their.Very.Long.ClassAccess do
begin
a.Value := b.Value;
…
end;
However this one should be implemented with 1) – because you can nest ‘with’ statements.
Noisrev said
Delphi NextGen compiler will NOT support ‘with’ statement, so it’s another reason to not use ‘with’ statement at all.
jpluimers said
Good to know that!
Chris said
One of Delphi’s features is that it stays backwards compatible. Why would they suddenly change that?
abouchez said
I suspect it will need something like the C# “using … { }” statement, for IDisposable proper handling.
“with….” will probably follow the “Oxygene” syntax.
Of course, this will be another backward compatibility problem.
A breaking issue.
Honestly, why not just go to C#, Oxygene, or FreePascal?
If the only unique aspect of Delphi if its IDE, there is no need to use it.
For instance, error insight is just broken – even with Delphi’s own RTL units.
If Delphi NextGen implies a garbage collector (with no ARC-like model), I do not think I will ever use nor support it.
Honestly, I do not have problem writing un-managed code, and handling my memory by hand.
If I want some GC and full object model, I will use C#, Java or any script language around.
I hope the NextGen would not push me to FreePascalCompiler and Lazarus.
Alister Christie said
There should be a compiler switch to warn about use the of with (or should it be an error). I made a video a while back on my take on with
http://learndelphi.tv/index.php?option=com_content&view=article&id=37:movie-19-hate-with-a-passion&catid=17:movie&Itemid=27
jpluimers said
Note sure about the compiler switch, but I surely love your video for these reasons:
– very to the point!
– it shows you exactly what I meant here: using with makes your code less future proof.
– the alternative
with
syntax you accidentally propose:begin
with MagicalFrame.Label1 do
begin
if .Font.Color = clBlue then
begin
.Font.Color := clRed;
.Caption := 'Red';
end
else
begin
.Font.Color := clBlue;
.Caption := 'Blue';
end;
end;
You might want to add this refactoring to your example, or even refactor the
ToggleLabel
method into theTFrame3
class:procedure TForm2.Button1Click(Sender: TObject);
begin
ToggleLabel(MagicalForm.Label1);
end;
procedure TForm2.ToggleLabel(const MagicalLabel: TLabel);
begin
if MagicalLabel.Font.Color = clBlue then
begin
MagicalLabel.Font.Color := clRed;
MagicalLabel.Caption := 'Red';
end
else
begin
MagicalLabel.Font.Color := clBlue;
MagicalLabel.Caption := 'Blue';
end;
end;
–jeroen
ObjectMethodology.com said
No more “with” magic. :0(
cricque said
With should never be used, it just creates problems. For all I care it should be removed from Pascal/Delphi.
Try debugging some code with a with statement, or multiple with statements … it’s so much fun
abouchez said
But sometimes, using “with… do” can be more readable.
If it is more readable: use it.
Otherwise, use a local variable.
Skydvrz said
Cricque,
I agree! While a With block might be more readable for the original programmer, they are a nightmare for the next poor bastard that has to try to figure it out. Which lines are fields of the with-parameter and which are external functions? Now I have to step through all the lines of the with block to see how it works. For extra credit (and grief) toss in some globals that have nothing to do with the with-param. Also, they are very difficult to debug since you cannot see the values easily. I remove all with-blocks that I see in legacy code – fabulously readable or not.
Chris said
I partly agree…
I think cases such as these are relatively rare, so I don’t think that just this makes a good argument not to use them. Having said that, I think they should only be used where it makes absolute sense and not just everywhere, in such cases as:
1. When readability is greatly improved (I have seen some good examples of this before).
2. Performance is improved.
Number 2 is for me the only real case for using them. I never use With statements when accesses first level properties.
jpluimers said
Can you give an example of 2. ?
Chris said
Example:
Object.Prop1.Prop1_1.Prop1_1_1
Object.Prop1.Prop1_1.Prop1_1_2
Object.Prop1.Prop1_1.Prop1_1_3
versus
with Object.Prop1.Prop1_1 do
begin
Prop1_1_1
Prop1_1_2
Prop1_1_3
end;
In the second code, Prop1_1 is referenced directly for all 3 calls. Where in the first code, the object and every parent propery is requalified on each call.
Another example:
Example 2: Performance & Readability
Employee.Address(“Home”).Address1 := ‘123 ABC Street’;
Employee.Address(“Home”).City := ‘Adelaide’;
Employee.Address(“Home”).State := ‘South Australia’;
versus
var
lAddress : TAddress;
begin
lAddress := Employee.GetAddress(‘Home’);
lAddress.Address1 := ‘123 ABC Street’;
lAddress.City := ‘Adelaide’;
lAddress.State := ‘South Australia’;
end;
versus
with Employee.Address(“Home”) do
begin
Address1 := ‘123 ABC Street’;
City := ‘Adelaide’;
State := ‘South Australia’;
end;
In this example I think the last code reads and formats slightly nicer than the second code, and with the performance benefit over the first code.
abouchez said
I do not think so.
There is no performance benefit of using “with … do” instead of a local variable.
In fact, the generated asm is the same as if a local variable was used: it will be coded using a register if possible, or a local pointer on the stack.
Nothing more, nothing less than a local variable.
There is a performance benefit only if you do not use a local variable, but re-evaluate a method within the loop.
But sometimes, using “with… do” can be more readable.
If it is more readable: use it.
Otherwise, use a local variable.
Chris said
For some simple usages of the With statement, yes I agree, however consider the following example:
type
TAddress = class
Address1 : String;
City : String;
State : String;
end;
type
TEmployee = class
private
mAddress : TAddress;
function GetAddress: TAddress;
public
constructor Create;
destructor Destroy; override;
property Address : TAddress read GetAddress;
end;
{ Employee }
constructor TEmployee.Create;
begin
mAddress := TAddress.Create;
end;
destructor TEmployee.Destroy;
begin
mAddress.Free;
inherited;
end;
function TEmployee.GetAddress: TAddress;
begin
Result := mAddress;
ShowMessage(‘Address Qualified’);
end;
{ TForm1 }
procedure TForm1.Button1Click(Sender: TObject);
var
lEmployee : TEmployee;
begin
lEmployee := TEmployee.Create;
try
lEmployee.Address.Address1 := ‘123 ABC Street’;
lEmployee.Address.City := ‘Adelaide’;
lEmployee.Address.State := ‘South Australia’;
finally
lEmployee.Free;
end;
end;
procedure TForm1.Button2Click(Sender: TObject);
var
lEmployee : TEmployee;
begin
lEmployee := TEmployee.Create;
try
with lEmployee.Address do
begin
Address1 := ‘123 ABC Street’;
City := ‘Adelaide’;
State := ‘South Australia’;
end;
finally
lEmployee.Free;
end;
end;
Calling Button1 Qualifies the Address getter 3 times, where calling Button2 only qualifies the Address getter once.
A. Bouchez said
to Jeroen:
… so your final code won’t work with XP?
The problem is not the “with” syntax here.
This is how the API structure was converted.
A dedicated NOTIFYICONDATAW2 or NOTIFYICONDATAW_EX record should have been defined.
Or using a function like this:
function PlatformSize(const IconData: NOTIFYICONDATAW): integer; inline;
begin
{$if CompilerVersion >= 20}
result := IconData.SizeOf();
{$else}
result := SizeOf(IconData);
{$ifend CompilerVersion >= 20}
end;
So in the end-user code:
with FIconData do
begin
cbSize := PlatformSize(FIconData);
Wnd := Self.Handle;
uID := $DEDB;
uFlags := NIF_MESSAGE or NIF_ICON or NIF_TIP;
hIcon := Application.Icon.Handle;
uCallbackMessage := WM_CAS400NTIcon;
StrCopy(szTip, PChar(Caption));
end;
Thanks to the “inline;” keyword, it will compile just fine.
And it will explicitly state that the size depends on the running platform.
jpluimers said
I think my code still works in XP, assuming that older versions of Delphi have the older version of the structure.
Please explain why you think otherwise.
Since Microsoft hasn’t introduced different names for the different versions of the
_NOTIFYICONDATAW
structure, I can understand that the Delphi team didn’t either.Note that in stead of depending on the compiler version, the
cbsize
value should be filled with the minimum Windows platform you intend to support.A. Bouchez said
So you assume that the version of Windows you are using for coding is the one on which the application will run?
This does not make sense, unless you are developing for yourself, and you are using Delphi 7 with Windows 2000.
:)
You need to adapt the size to the platform it runs on, not the compiler it is using.
See official documentation:
You can maintain application compatibility with all Shell32.dll versions while still using the current header files by setting the size of the NOTIFYICONDATA structure appropriately. Before you initialize the structure, use DllGetVersion to determine which Shell32.dll version is installed on the system and initialize cbSize with one of these values:
Shell32.dll Version cbSize
6.0.6 or higher (Windows Vista and later) sizeof(NOTIFYICONDATA)
6.0 (Windows XP) NOTIFYICONDATA_V3_SIZE
5.0 (Windows 2000) NOTIFYICONDATA_V2_SIZE
Versions lower than 5.0 NOTIFYICONDATA_V1_SIZE
http://msdn.microsoft.com/en-us/library/windows/desktop/bb773352(v=vs.85).aspx
jpluimers said
Which is basically the final point I made in my previous reply.
I like that we are along the same line of thought here (:
Ruurd Pels said
Uhm, why blame the language or the compiler when the real problem is some idiot API change that breaks your code. I mean really. What flaming idiot makes this kind of changes in a struct expecting that a single compile will keep working over multiple versions of the API? Wrap the thing in a library for god sake.
jpluimers said
Because not using with would reveal the underlying problem sooner.
But I agree with you that API changes should be done with care. This one should have had a bit more care.
Jim McKeeth said
I agree with you, which is why I really prefer the Oxygene “with” syntax. It has all of the benefits with none of the detriments. http://wiki.oxygenelanguage.com/en/With_(keyword)
François said
Agreed 100%…