4

I have several TPanels that are populated with buttons at runtime. However the code below that i use to free my buttons from their parent panels sometimes generates access violation errors.

procedure TfrmTakeOrder.FreeItemButtons(buttons : array of TButton);
var
  cnt,i : integer;
begin

  for i := 0 to gridLayoutItems.ControlCount - 1 do
    begin
      buttons[i].Free;
      buttons[i] := nil;
    end;

end;

Is there a better way to do this?Please keep in mind that other Panels have buttons too and I would like to have a "localised" freeing of the buttons that wount intefer with other panels.

8 Answers 8

7

It looks to me like you are trying to remove all buttons from a TPanel and that panel only contains buttons.

Try this:

while gridLayoutItems.ControlCount > 0 do
  gridLayoutItems.Controls[0].Free;
1
  • 2
    Make sure you give up on your own button array though. Otherwise you have references to already freed memory in your array and run into access violations if you try to use them. Jun 9, 2011 at 12:10
3

If:

  • that panel has only buttons, and
  • your array has all the buttons on several panels.

Then use:

var
  I: Integer;
  Button: TControl;
begin
  for I := GridLayoutItems.ControlCount - 1 downto 0 do
  begin
    Button := GridLayoutItems.Controls[I];
    Button.Free;
    { Find the index of Button in the Buttons array };
    { Erase that item from the array };
  end;
end;

But in this scenario, it is much more handy to have a TList for Buttons instead of an array, becasue then the code becomes:

var
  I: Integer;
  Button: TControl;
begin
  for I := GridLayoutItems.ControlCount - 1 downto 0 do
  begin
    Button := GridLayoutItems.Controls[I];
    Button.Free;
    Buttons.Remove(Button);
  end;
end;
1

Maybe it would be better to use Length(buttons) - 1 instead of gridLayoutItems.ControlCount - 1 which could be different.

4
  • Not Maybe, definitively, because even if gridLayoutItems happens to only contain those buttons, freeing the buttons makes ControlCount decrease: you'll stop half-way! And don't bother setting buttons[i] := nil' because buttons is a parameter to the function, the new values are lost when the function returns. Jun 9, 2011 at 10:55
  • I wasn't aware that dynamic arrays are passed by copy instead of reference. Good point.
    – ba__friend
    Jun 9, 2011 at 11:26
  • 2
    You won't stop half-way, @Cosmin. The for-loop bounds are computed once at the start of the loop; they're not re-evaluated as the loop runs. Jun 9, 2011 at 13:38
  • @Ba__fiend, dynamic arrays are passed by reference. But that's not a dynamic array. It's an open array. Jun 9, 2011 at 13:39
1

You have to correct the bounds of your for-loop like the other answers state. One more thing:

How do you create the buttons? If you create the buttons with an owner, i.e.

buttons [i] := TButton.Create (Panel);

then you must not free them manually. The owner takes care of that. In this case just use

buttons [i] := TButton.Create (nil);
2
  • 5
    it's perfectly OK to do buttons[i] := TButton.Create(Whatever) and then free the button. When the button is freed, it's owner gets a notification to remove it from it's list of owned objects so it doesn't free it again. That is, if the buttons are freed because the form needs to be changed at runtime: if the code to free the buttons is called from the form's destructor, than it's pointless and potentially dangerous. Jun 9, 2011 at 11:00
  • @Cosmin: thanks for the heads-up, I didn't know that. Always thaught it is either Owner or manual Free. Jun 9, 2011 at 11:01
0

Yes. Use for i := Low(buttons) to high(Buttons) do

2
  • I have several TPanels on the form.Wount this get the count of the buttons in the other panels as well?
    – davykiash
    Jun 9, 2011 at 10:57
  • If you store the buttons in one array it will of course remove the buttons from all panels. Jun 9, 2011 at 10:59
0

In more recent versions of Delphi (like XE5), Free does not exist any more. I solved the problem using Destroy instead of Free.

0

If you want to free only buttons from parent panel:

var i : integer
begin
i := 0;
while Panel1.ControlCount > i do
  if Panel1.Controls[i] is TButton then
     Panel1.Controls[i].Free
  else inc(i);
end;
0

you can use this code

for i := 0 to ComponentCount-1 do
 begin
  if ( Components[ i ] is TPanel ) then
  (Components[ i ] as TPanel ).free;
 end;

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.