Delphi TThread descendant return result

579 views Asked by At

SITUATION. I have created an unit with some classes to solve algebra stuff (congruences and systems), I am showing you the code:

type
 TCongrError = class(Exception)
 end;

type
 TCongruence = class(TComponent)
  //code stuff
  constructor Create(a, b, n: integer); virtual;
 end;

type
 TCongrSystem = array of TCongruence;

type
 TCongruenceSystem = class(TThread)
  private
   resInner: integer;
   FData: TCongrSystem;
   function modinv(u, v: integer): integer; //not relevant
  protected
   procedure Execute; override;
  public
   constructor Create(data: TCongrSystem; var result: integer; hasClass: boolean);
 end;

I have decided to use TThread because this class has an Execute method that could take some time to finish due to the length of the parameters passed to the constructor. Here's the implementation:

constructor TCongruenceSystem.Create(data: TCongrSystem; var result: integer; hasClass: boolean);
begin

 inherited Create(True);
 FreeOnTerminate := true;

 FData := data;
 setClass := hasClass;
 resInner := result;

end;

procedure TCongruenceSystem.Execute;
var sysResult, i, n, t: integer;
begin

 sysResult := 0;
 n := 1;

 //computation

 Queue( procedure
        begin
         ShowMessage('r = ' + sysResult.ToString);
         resInner := sysResult;
        end );

end;

PROBLEM

If you look at the Queue you see that I am using (just as test) the ShowMessage and it is showing the correct value of sysResult. The second line by the way has some problems that I cannot understand.

The constructor has var result: integer so I can have side-effect from the passed variable and then I can assign resInner := result;. At the end (in the Queue) I am giving resInner the value of sysResult and I expect result to be updated too due to the side effect of var. Why doesn't this happen?

I have made another test changing the constructor like this:

constructor TCongruenceSystem.Create(data: TCongrSystem; result: TMemo; hasClass: boolean);
//now of course I have resInner: TMemo

And changing the Queue to this:

Queue( procedure
        begin
         ShowMessage('r = ' + sysResult.ToString);
         resInner.Lines.Add(sysResult.ToString);
        end ); //this code now works properly in both cases! (showmessage and memo)

In the constructor I am passing TMemo which is a reference and ok, but isn't the original var result: integer passed as reference too? Why then it doesn't work?

I want to do this because I'd like to do something like this:

 //I put var a: integer; inside the public part of the TForm
 test := TCongruenceSystem.Create(..., a, true);
 test.OnTerminate := giveMeSolution;
 test.Start;
 test := nil;

Where giveMeSolution is just a simple procedure that uses the variable a containing the result of the system. If this is not possible what could I do? Basically the result at the end of Execute is just an integer number that has to be passed to the main thread.

I have read about ReturnValue but I am not sure how to use it.

2

There are 2 answers

5
Remy Lebeau On BEST ANSWER

Basically the result at the end of Execute is just an integer number that has to be passed to the main thread.

I have read about ReturnValue but I am not sure how to use it.

Using the ReturnValue property is very easy:

type
  TCongruenceSystem = class(TThread)
    ... 
  protected
    procedure Execute; override;
  public
    property ReturnValue; // protected by default
  end;

procedure TCongruenceSystem.Execute;
var
 ...
begin
  // computation
  ReturnValue := ...;
end;

test := TCongruenceSystem.Create(...);
test.OnTerminate := giveMeSolution;
test.Start;

....

procedure TMyForm.giveMeSolution(Sender: TObject);
var
  Result: Integer;
begin
  Result := TCongruenceSystem(Sender).ReturnValue;
  ...
end;
1
J... On

Let's assume a class field FFoo : integer; ;

 procedure TFoo.Foo(var x : integer);
 begin
   FFoo := x;
 end;

Here what you are doing is assigning the value of x to FFoo. Inside the method Foo you are free to modify the value of the variable passed in as x but integers are otherwise value types that are copied on assignment. If you want to keep a reference to an external integer variable you would need to declare FFoo (or, in your case, resInner) as a PInteger (pointer to an integer). For example (simplifying) :

TCongruenceSystem = class(TThread)
  private
    resInner: PInteger;       
  protected
    procedure Execute; override;
  public
    constructor Create(result: PInteger);
end;

where

constructor TCongruenceSystem.Create(result: PInteger);
begin    
  inherited Create(True);
  FreeOnTerminate := true;      
  resInner := result;    
end;

which you would call as test := TCongruenceSystem.Create(@a); and assign:

 { ** See the bottom of this answer for why NOT to use }
 {    Queue with FreeOnTerminate = true **             }
 Queue( procedure
    begin
      ShowMessage('r = ' + sysResult.ToString);
      resInner^ := sysResult;
    end );

The reason it works with TMemo is that classes are reference types - their variables do not hold values but rather point to the address of the object in memory. When you copy a class variable you are only copying a reference (ie: a pointer) whereas for value types the contents of the variable are copied on assignment.


With that said, there's nothing stopping you from keeping the argument typed as var x : integer and taking a reference in your constructor :

constructor TCongruenceSystem.Create(var result: Integer);
begin    
  inherited Create(True);
  FreeOnTerminate := true;      
  resInner := @result;   {take the reference here}   
end;

but this gives the caller the impression that once the constructor is complete that you have made any modifications to the variable you intend to and they are free to dispose of the integer. Passing explicitly as PInteger gives the caller a hint that your object will keep a reference to the integer they provide and that need to ensure the underlying variable remains valid while your class is alive.

And... with all that said, I still fundamentally don't like this idea. By taking in a variable reference like this you are offloading an atypical lifetime management issue to the caller. Passing pointers is best done in place where they are used at the point of transfer only. Holding onto a foreign pointer is messy and it's too easy for mistakes to happen. A far better approach here would be to provide a completion event and have the consumer of your class attach a handler.

For example :

  { define a suitable callback signature }
  TOnCalcComplete = procedure(AResult : integer) of object;

  TCongruenceSystem = class(TThread)
  private
    Fx, Fy : integer;
    FOnCalcComplete : TOnCalcComplete;
  protected
    procedure Execute; override;
  public
    constructor Create(x,y: integer);
    property OnCalcComplete : TOnCalcComplete read FOnCalcComplete write FOnCalcComplete;
  end;

constructor TCongruenceSystem.Create(x: Integer; y: Integer);
begin
  inherited Create(true);
  FreeOnTerminate := true;
  Fx := x;
  Fy := y;
end;

procedure TCongruenceSystem.Execute;
var
  sumOfxy : integer;
begin
  sumOfxy := Fx + Fy;
  sleep(3000);        {take some time...}
  if Assigned(FOnCalcComplete) then
    Synchronize(procedure
                begin
                  FOnCalcComplete(sumOfxy);
                end);
end;

Which you would then call as :

{ implement an event handler ... }
procedure TForm1.CalcComplete(AResult: Integer);
begin
  ShowMessage(IntToStr(AResult));
end;

procedure TForm1.Button1Click(Sender: TObject);
var
  LCongruenceSystem : TCongruenceSystem;
begin
  LCongruenceSystem := TCongruenceSystem.Create(5, 2);
  LCongruenceSystem.OnCalcComplete := CalcComplete;  { attach the handler }
  LCongruenceSystem.Start;
end;

You'll also notice that I used Synchronize here instead of Queue. On this topic, please have a read of this question (I'll quote Remy...):

Ensure all TThread.Queue methods complete before thread self-destructs

Setting FreeOnTerminate := True in a queued method is asking for a memory leak.