Watch, Follow, &
Connect with Us

For forums, blogs and more please visit our
Developer Tools Community.


Welcome, Guest
Guest Settings
Help

Thread: Is this procedure safe?


This question is answered.


Permlink Replies: 4 - Last Post: Oct 29, 2015 12:20 PM Last Post By: Francisco Alvar...
Francisco Alvar...

Posts: 83
Registered: 11/10/06
Is this procedure safe?  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Oct 29, 2015 7:25 AM
In an industrial application, this procedure executes every second:
{begin code}
procedure TMain.ActivateLeds(Value: Int64);
var
sBinary: String;
TLed: TILLed;
i: Integer;
begin
sBinary := IntToBin(Value);
for i := 64 DownTo 1 do
begin
TLed := (FindComponent('ILLed' + IntToStr(64-i)) as TILLed);
if Assigned(TLed) then
TLed.Value := (SBinary[i] = '1');
end;
end;
{end code}
I'm worried about TLed object can persist as it is not freed.
Do I need to release it after each iteration (for i := ...) or at the end of the procedure?
Best regards
Francisco Alvarado
Robert Triest

Posts: 687
Registered: 3/24/05
Re: Is this procedure safe?
Helpful
Click to report abuse...   Click to reply to this thread Reply
  Posted: Oct 29, 2015 7:39 AM   in response to: Francisco Alvar... in response to: Francisco Alvar...
TLed := (FindComponent('ILLed' + IntToStr(64-i)) as TILLed);
TLed has only a pointer to the component when found.
You didn't create a new TLed here.
Probably your TILLed components will be freed when their parent closes.

p.s. use twice "code" instead of "begin code" and "end code"

Edited by: Robert Triest on Oct 29, 2015 3:42 PM
Francisco Alvar...

Posts: 83
Registered: 11/10/06
Re: Is this procedure safe?  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Oct 29, 2015 12:19 PM   in response to: Robert Triest in response to: Robert Triest
Robert Triest wrote:
TLed := (FindComponent('ILLed' + IntToStr(64-i)) as TILLed);
TLed has only a pointer to the component when found.
You didn't create a new TLed here.
Probably your TILLed components will be freed when their parent closes.

p.s. use twice "code" instead of "begin code" and "end code"

Edited by: Robert Triest on Oct 29, 2015 3:42 PM
Thank you Robert!
Remy Lebeau (Te...


Posts: 9,447
Registered: 12/23/01
Re: Is this procedure safe?
Correct
Click to report abuse...   Click to reply to this thread Reply
  Posted: Oct 29, 2015 10:55 AM   in response to: Francisco Alvar... in response to: Francisco Alvar...
Francisco wrote:

In an industrial application, this procedure executes every second:

FindComponent() searches for the requested component every time it is called.
To greatly speed up the code, I would suggest putting your TILLed components
into an array when the Form is first created, and then just loop through
the array when needed, eg:

type
  TMain = class(TForm)
    procedure FormCreate(Sender: TObject);
    ...
  private
    Leds: array[0..63] of TILLed;
    ...
  end;
 
procedure TMain.FormCreate(Sender: TObject);
var
  i: Integer;
begin
  for i := 0 to 63 do
    Leds[i] := FindComponent('ILLed' + IntToStr(i)) as TILLed;
end;
 
procedure TMain.ActivateLeds(Value: Int64);
var
  sBinary: String;
  i: Integer;
begin
  sBinary := IntToBin(Value);
  for i := 0 To 63 do
    Leds[i].Value := (SBinary[64-i] = '1');
end;


I'm worried about TLed object can persist as it is not freed.

All you are doing is locating and acccessing pointers to existing objects.
You are not allocating anything, so there is nothing to free.

Do I need to release it after each iteration (for i := ...) or at
the end of the procedure?

No, and no.

--
Remy Lebeau (TeamB)
Francisco Alvar...

Posts: 83
Registered: 11/10/06
Re: Is this procedure safe?  
Click to report abuse...   Click to reply to this thread Reply
  Posted: Oct 29, 2015 12:20 PM   in response to: Remy Lebeau (Te... in response to: Remy Lebeau (Te...
Remy Lebeau (TeamB) wrote:
Francisco wrote:

In an industrial application, this procedure executes every second:

FindComponent() searches for the requested component every time it is called.
To greatly speed up the code, I would suggest putting your TILLed components
into an array when the Form is first created, and then just loop through
the array when needed, eg:

type
  TMain = class(TForm)
    procedure FormCreate(Sender: TObject);
    ...
  private
    Leds: array[0..63] of TILLed;
    ...
  end;
 
procedure TMain.FormCreate(Sender: TObject);
var
  i: Integer;
begin
  for i := 0 to 63 do
    Leds[i] := FindComponent('ILLed' + IntToStr(i)) as TILLed;
end;
 
procedure TMain.ActivateLeds(Value: Int64);
var
  sBinary: String;
  i: Integer;
begin
  sBinary := IntToBin(Value);
  for i := 0 To 63 do
    Leds[i].Value := (SBinary[64-i] = '1');
end;


I'm worried about TLed object can persist as it is not freed.

All you are doing is locating and acccessing pointers to existing objects.
You are not allocating anything, so there is nothing to free.

Do I need to release it after each iteration (for i := ...) or at
the end of the procedure?

No, and no.

--
Remy Lebeau (TeamB)
Thank you Remy!
Legend
Helpful Answer (5 pts)
Correct Answer (10 pts)

Server Response from: ETNAJIVE02