Change of coding style/structure..

[Ian Branch 127]

Hi Team,

For years (Decades?) I have used the following coding style/structure, or similar..

procedure MainForm.ButtonClick(Sender: Object)
var
myForm: TMyForm;
begin
MyForm :- TMyForm.Create(nil);
try
	MyForm.ShowModal;
finally
	FreeAndNil(MyForm);
end;

MyForm is not declared as a var in the MyForm.pas.

I recently came across a change in this style/structure that results in this..

procedure MainForm.ButtonClick(Sender: Object)
begin
with TMyForm.Create(nil) do
try
	ShowModal;
finally
	Free;
end;

Leaving aside the use of 'FreeAndNil()' and 'with' discussions, to me this is a simpler structure, easier to code and read, and it works.  Those things make my life easier.. 😉

So To my question - As you can see in the first code I use FreeAndNil(), as much for peace of mind, imagined or real, as anything else.

Is there a way to implement the equivalent in the second code structure?

 

Regards & TIA,

Ian

[Dave Nottage 557]

No comment. Oh wait.. that was a comment 😉

 

[Lajos Juhász 293]

The same from me, please d.... c.d. th.. w....

[Lars Fosdal 1792]

I go out of my way to avoid using with.

Why would you need to explicitly set a local variable nil? One that just has been free'd and is about to be non-existent?

[David Heffernan 2345]

If using a local variable with the basic try finally, Create, ShowModal, Free pattern is complicated and requires simplification, then I think you may have deeper problems. 

[Ian Branch 127]

Dave - 🙂

Lajos - W.. n..?

Lars - I'm not exactly sure which code block you are referring to?

David - It isn't complicated.  It can just be easier.

[David Heffernan 2345]

4 minutes ago, Ian Branch said:

It can just be easier

And then you need to call a few more methods and then you risk with hell. Or you need to pass the form reference to another method and you don't have it any more. So you change back to local variable pattern. Now have you multiple different patterns for the same task. Which is fine if the different patterns bring significant benefit. But they don't. So use a single unified pattern. 

[Stano 143]

I recently read that only .Free should  be used to release the form

[FPiette 383]

You should stay with the original version for the reasons David gave.

Additionally, pay attention to code indentation like this:

procedure TMainForm.ButtonClick(Sender: Object)
var
    myForm: TMyForm;
begin
    MyForm := TMyForm.Create(nil);
    try
        MyForm.ShowModal;
    finally
        FreeAndNil(MyForm);
    end;
end;

 

Edited October 10, 2022 by FPiette

[FPiette 383]

1 hour ago, Stano said:

I recently read that only .Free should  be used to release the form

You are partially right. This recommendation is for destructing a form in the context of an event handler of that form. In that case you need to call Release which defers the actual destruction after the event is terminated. This doesn't apply in this case.

[Lajos Juhász 293]

2 hours ago, Ian Branch said:

Lajos - W.. n..?

David already said everything. With can cause that you access something that you doesn't think (new property, field, method). Calling FreeAndNil for a local object has no reason it's just a classic code smell.

 

Your aim should be to write clean and maintanable code that cannot be achieved using with.

[FPiette 383]

4 hours ago, Lajos Juhász said:

Calling FreeAndNil for a local object has no reason it's just a classic code smell.

I like to use FreeAndNil for the same reason David explained for the rest : One day you make your procedure more complex, adding some code lines after the FreeAndNil and yet use the freed variable. Calling FreeAndNil instead of Free will make you quickly discover (AV° the error you made. The price is very low for this security.

 

[Dalija Prasnikar 1396]

11 hours ago, Ian Branch said:

Leaving aside the use of 'FreeAndNil()' and 'with' discussions, to me this is a simpler structure, easier to code and read, and it works.  Those things make my life easier.. 😉

I don't think you can avoid that 

 

'with' has many pitfalls and replacing code that does not use 'with' with code using 'with' is not something I would recommend doing. 

11 hours ago, Ian Branch said:

So To my question - As you can see in the first code I use FreeAndNil(), as much for peace of mind, imagined or real, as anything else.

Is there a way to implement the equivalent in the second code structure?

You have to ask yourself what is the purpose of FreeAndNil? In the case of local variable the reason (regardless whether we consider it valid or not) is to prevent accidental access of the object instance after it has been released. In such cases we hope that accessing nil reference would crash immediately revealing the obvious bug in the code.

 

However, when you use 'with' you no longer have explicit reference to the object that you can accidentally access outside the 'with' block. Yes, if you are really sloppy, you can still call Free first and that call some other code if call to Free is not the last thing you do in the 'with' block. Main point is compiler hides the reference, there is nothing you can nil here, and compiler will certainly not access the instance outside the 'with' block by mistake.

[Ian Branch 127]

All,

I thank you for your constructive responses.

Whilst I understand the aversion to using 'With', I do believe it has its place.

I don't see the issue with its use here within a single code block opening a from from a button or menu selection.

Yes, at a later date there may be the need to add additional code into the block, at that point in time is when the use of With becomes a serious question.

Having said the above, I will in any case stick with my original code structure.

Again, thank you all,

Regards,

Ian

[dummzeuch 1505]

12 hours ago, FPiette said:

You are partially right. This recommendation is for destructing a form in the context of an event handler of that form. In that case you need to call Release which defers the actual destruction after the event is terminated. This doesn't apply in this case.

I don't think @Stano was referring to the method TForm.Release.

[Ian Branch 127]

Hi Team,

Never heard of, nor seen TForm.Release in use.  Intriguing

Would someone care to demonstrate its use/application with a bit of code??

 

Ian

[haentschman 92]

Quote

to using 'With', I do believe it has its place.

In the modern Delphi...always NO.

see uses TRect - https://stackoverflow.com/questions/71419/why-should-i-not-use-with-in-delphi ...and other.

procedure MainForm.ButtonClick(Sender: Object)
begin
with TMyForm.Create(nil) do
try
	ShowModal;
finally
	Free;
end;

...this means a rebuild of the code if complexity increases...why not "right" with instance variable.

...and the variable display in the debugger does not work.

 

Quote

The biggest being that with actually reduces readability. NOTE: Less code (using with) does not automatically improve readability. It reduces it because it's no longer explicit what the scope of identifiers within a with block refer to.

 

I am out of this discussion...

Edited October 11, 2022 by haentschman

[microtronx 38]

I don't like "with" and do try to avoid it all the time!

[Ian Branch 127]

Hi All,

The passion behind not using 'with' is interesting.

Also interesting is that I see no advocates.  Yes, yes, you will say "And the reason is....". 🙂

To me it is just another tool in the tool box of the language.

I can see and understand where concerns may be but as with any tool, used correctly, there is no issue.

So, I wonder where you all stand on something like this...

    with TTaskDialog.Create(Self) do
      try
        Title := 'Warning!';
        Text := 'Relevant/selected Records prior to ' + sArchivedDate + ' have been Archived!' + #13 + 'This may make the figures inaccurate!';
        CommonButtons := [tcbOk];
        Execute;
      finally
        Free;
      end;

Should it become..

    MyDialog := TTaskDialog.Create(Self);
      try
        MyDialog.Title := 'Warning!';
        MyDialog.Text := 'Relevant/selected Records prior to ' + sArchivedDate + ' have been Archived!' + #13 + 'This may make the figures inaccurate!';
        MyDialog.CommonButtons := [tcbOk];
        MyDialog.Execute;
      finally
        MyDialog.Free;
      end;

Or perhaps..

    MyDialog := TTaskDialog.Create(Self);
    MyDialog.Title := 'Warning!';
    MyDialog.Text := 'Relevant/selected Records prior to ' + sArchivedDate + ' have been Archived!' + #13 + 'This may make the figures inaccurate!';
    MyDialog.CommonButtons := [tcbOk];
    try
      MyDialog.Execute;
    finally
      MyDialog.Free;
    end;

Regards,

Ian

[haentschman 92]

Hi...

 

Quote

 

MyDialog := TTaskDialog.Create(Self);
try
  MyDialog.Title := 'Warning!';
  MyDialog.Text := 'Relevant/selected Records prior to ' + sArchivedDate + ' have been Archived!' + #13 + 'This may make the figures inaccurate!';
  MyDialog.CommonButtons := [tcbOk];
  MyDialog.Execute;
finally
  MyDialog.Free;
end;

 

...this is right. 😉

 

Quote

To me it is just another tool in the tool box of the language.

...but the language has also changed. "with" is 90's. The "new" TRect was imho the first problem in this case. The structure of TRect was changed. Some "with" had the false scope...AccessViolation.   

 

The biggest problem with "with" is:

Quote

One annoyance with using with is that the debugger can't handle it. So it makes debugging more difficult.

with TTaskDialog.Create(Self) do
try
  Title := 'Warning!';
  Text := GetTextFromFunction;
  CommonButtons := [tcbOk];
  Execute;
finally
  Free;
end;

...and now you set a breakpoint on "Excecute". Then read the variable content from "Text" under the mouse...you see nothing. 

Quote

By the way, it will allow easier debugging: you can put a breakpoint, then point your mouse on Obj or Nested directly to get the internal values.

More Info ( June 26, 2007 )

https://blog.marcocantu.com/blog/with_harmful.html

 

Edited October 12, 2022 by haentschman

[FPiette 383]

8 hours ago, Ian Branch said:

Should it become..

    MyDialog := TTaskDialog.Create(Self);
      try
        MyDialog.Title := 'Warning!';
        MyDialog.Text := 'Relevant/selected Records prior to ' + sArchivedDate + ' have been Archived!' + #13 + 'This may make the figures inaccurate!';
        MyDialog.CommonButtons := [tcbOk];
        MyDialog.Execute;
      finally
        MyDialog.Free;
      end;

Yes, it should. I always code like that and when taking code from elsewhere, I rewrite it like that.

[dummzeuch 1505]

8 hours ago, Ian Branch said:

The passion behind not using 'with' is interesting.

Once you have tried to debug code with with, sometimes even with multiple variables listed in the statement ("with a, b, c do") and even nested, you will start to dislike it even for small parts of code. And then there are the issues with new properties and fields being introduced in ancestor classes which break the code in a with block because rather an accessing a local variable all of a sudden the code accesses a property brought into scope by a with.

 

That's where my "passion" comes from. My time is too valuable to waste it with problems introduced by being too lazy for typing an identifier. If you really want to save some keystrokes, at least declare a local variable, even inline if your compiler supports it:

begin
  var Items := ListView1.Items.
  for i := 0 to Items.Count - 1 do begin
    Process(Items[i].Caption);
  end;
end;

 

[Sherlock 663]

There should actually be a refactoring tool to "de-with" a portion of code. That would be really cool.

And something else is in the toolbox you would not dream to use:

Goto

 

[dummzeuch 1505]

13 minutes ago, Sherlock said:

There should actually be a refactoring tool to "de-with" a portion of code. That would be really cool.

And something else is in the toolbox you would not dream to use:

Goto

I actually used Goto once and I still think it was better than all alternatives in that one specific case. But in general I agree.

 

But do we really need yet another discussion on this? And Exit, and Continue and Break and .... FreeAndNil ?

I think everything has already been said about these, multiple times, actually too many times.

[Sherlock 663]

5 minutes ago, dummzeuch said:

But do we really need yet another discussion on this? And Exit, and Continue and Break and .... FreeAndNil ?

Nonono, of course not. I just wanted to weaken the "it's just another tool in the box" argument.