Delphi how to free an array of dynaically instantiated buttons?

Question

I have a problem freeing a array of buttons i created on frmTransaction.Show I get an invalid operation error when the from shows again. And when I run the program with a different user it says there are items already with that name :) This code is the only part of my program where I free memory

SetLength(btnSale,iTrans);
for i := 1 to iTrans do
begin
  readln(tPos,sPos);
  iPos := Pos(';',sPos);                         //Gets positions of buttons
  sTop := Copy(sPos,1,iPos-1);
  sLeft := Copy(sPos,iPos+1,length(sPos));

  btnSale[i] := TButton.Create(gbxSales);
  with btnSale[i] do
    begin
     Parent := gbxSales;
     name := 'Transaction' +
              IntToStr(dmdata.tblTransactions['TransactionID']);  //Creates buttons that represent Transactions
     Caption := 'Sale ' + IntToStr(i);
     Width := 153;
     Height := 97;
     Top := StrToInt(sTop);
     left := strToInt(sleft);
     show;
     onClick := ClickSale;
    end;
  dmdata.tblTransactions.Next;
end;


procedure TfrmTransactions.FormHide(Sender: TObject);
var
  i : integer;
begin
  for i := low(btnSale) to high(btnSale) do  //frees dynamically created objects
  begin
    btnSale[i].Free;
    btnSale[i] := nil;
  end;
end;

You've removed key parts of your program. We can't see where the first section of code lives. There's no need to give these controls names. We don't know what "with a different user" means. Please cut this down and make an MCVE. — David Heffernan, Aug 26 '15 at 10:29
Are you sure, you want to create/free the buttons in `FormShow`/`FormHide`? Why not creating them in `FormCreate`? Then you wouldn't need to free them explicitly because you defined an owner when creating them. — René Hoffmann, Aug 26 '15 at 10:31
Have you tried setting a breakpoint in the FormHide to check if the freeing has actually happened ? Also, when is the creating called ? We cannot see that from your code, please show all relevant code. — GuidoG, Aug 26 '15 at 10:35
I noticed that you give the buttons a parent: bgxSales. If this parent gets freed somewhere that could generate this error when you try to free your buttons in the FormHide — GuidoG, Aug 26 '15 at 10:40
If you are creating the button with an owner, `TButton.Create(gbxSales)` just free `gbxSales` and you are good to go. It will automatically destroy your buttons — EProgrammerNotFound, Aug 26 '15 at 11:36

score 4 · Answer 1 · answered Aug 26 '15 at 11:19

4

SetLength function sets the length of a dynamic array. The high member will be MyArray[Length(MyArray)-1] (High function is used instead in the second part of your code). You code is addressing a member btnSale[i] and so it is not doing anything with member #0 and is addressing member btnSale[Length(btnSale)] that is above the high border.

Try to use this.

    SetLength(btnSale,iTrans);
    for i := 0 to iTrans-1 do
// or this for i := low(btnSale) to high(btnSale) do
    begin
    ...
      btnSale[i] := TButton.Create(gbxSales);
      with btnSale[i] do
        ...

    end;

answered Aug 26 '15 at 11:19

asd-tm

3,381
2
24
41

Wow, so obvious, cannot believe I did not see this. – GuidoG Aug 26 '15 at 11:36
... so with `for i := 1 to iTrans do `, `btnSale[i] := TButton.Create(gbxSales);` should raise an *index out of bound exception* when `i` reaches `iTrans`, shouldn't it? – fantaghirocco Aug 26 '15 at 12:14
@fantaghirocco, It seems so, but we do not see the context of the code calling. What if it is somewhere inside `try except end`? Any way. The index is set unproperly and it is necesssary to correct this mistake before we can proceed to the others. – asd-tm Aug 26 '15 at 12:26
I agree with your answer and comment but I was wondering why the app didn't raise the exception before: you may be right also with the `try`-`catch` block – fantaghirocco Aug 26 '15 at 12:28
@fantaghirocco Hope the author will explain us. It is interesting indeed:) – asd-tm Aug 26 '15 at 12:29
@fantaghirocco It will only raise an exception if compiled with range checking turned on. Otherwise it will simply index the array out of bounds, possibly crashing, possibly corrupting data, and possibly just doing nothing at all. (This is why it is important to not turn off range checks without a very solid understanding of what you are doing and a very good reason why). – J... Aug 26 '15 at 12:45
@fant Error only raised if range checking enabled. This is disabled by default for reasons inexplicable. – David Heffernan Aug 26 '15 at 12:45
@DavidHeffernan I suppose we can hypothesize that range checking was sufficiently expensive in the mid-late 90s (and developers somewhat more fastidious) that having it off by default for Release builds made sense. I'm not sure either of those arguments apply today. – J... Aug 26 '15 at 12:49
I misundestood `$r` thinking that range was checked at compile time. Thank you both – fantaghirocco Aug 26 '15 at 12:51
1

@J... Default off for Release is still reasonable. That's how I roll. But it's insane to default off for debug build. On the one hand Emba say they want to introduce ARC to make programming easier for novices. But then they won't enable range checking, overflow checking or typed address. – David Heffernan Aug 26 '15 at 12:53
@DavidHeffernan Agreed. Off for debug only makes sense if you are certain the code is fine in that regard *and* that it is significantly impacting performance and productivity to debug with checks on. On a 486 I could see that being the case... today, forget about it. – J... Aug 26 '15 at 12:59
I think range and overflow checks can always be on, even in Release code. I could have missed something in Debug mode, so at runtime, I rather see a range error happen than a program going on with false (e.g. wrapped around) data, or data accessed from an invalid index, or worse, like unnoticed memory corruption, etc. But I do understand those who rather turn it of at Release. – Rudy Velthuis Aug 26 '15 at 20:06
@Rudy That one comes down to perf. If the perf implication is not significant then more checking is good, even in release. For my needs, perf implication is too much. – David Heffernan Aug 26 '15 at 20:26
Perf? Do you mean perfection, and if so, why would having checks on be "imperfect"? If not, what do you mean? – Rudy Velthuis Aug 26 '15 at 21:14
And what is better then - to use a dynamic or static array? – asd-tm Aug 27 '15 at 07:44
@asd-tm That choice is determined by whether or not you know the length at compile time – David Heffernan Aug 27 '15 at 08:28
@David-Heffernan. I am asking becausse of [this](http://stackoverflow.com/a/1593719/5043424). In other words `VarName: array [0..n] of byte;` is not equal to `VarName: array of byte;... SetLength(VarName, n-1)`. – asd-tm Aug 27 '15 at 08:44
@asd-tm Well, no. For a start, the static array has two more elements. But which is better to use really depends on the situation. I use both static and dynamic arrays regularly. They both have their place. – David Heffernan Aug 27 '15 at 08:48
@David-Heffernan. I entirely agree with you. I decided to remind this aspect as soon as we are discussing such fine performance issues as `$R` directive. I suppose that the performance contribution weight of both aspects might be similar and in general can be neglected in modern PCs. – asd-tm Aug 27 '15 at 09:36
Ah that dreaded performance. IME, performance is only important in a small part of the code, and that would be the only code where I'd turn them off. – Rudy Velthuis Aug 27 '15 at 11:09
And note that [I find performance important too](http://stackoverflow.com/questions/32084204/problems-with-adc-sbb-and-inc-dec-in-tight-loops-on-some-cpus). – Rudy Velthuis Aug 27 '15 at 11:34

Delphi how to free an array of dynaically instantiated buttons?

1 Answers1