Why is my global variable "unavailable" when debugging?

Question

I am creating an application containing about 30 forms. I need to manage sessions, so I would like to have the global variable LoggedInUser accessible from all forms. I read the David Heffernan post on global variables and how to avoid them, but I thought it would be easier to have a global user variable, rather than 30 forms that have their own user variable.

So I have a unit: GlobalVars

unit GlobalVars; interface uses User; // I defined my TUser class in a unit called User var LoggedInUser: TUser; implementation initialization LoggedInUser:= TUser.Create; finalization LoggedInUser.Free; end. 

Then in my procedure LoginForm LoginBtnClick I do:

 unit FormLogin; interface uses [...],User; type TForm1 = class(TForm) [...] procedure LoginBtnClick(Sender: TObject); private { Déclarations privées } public end; var Form1: TForm1; AureliusConnection : IDBConnection; implementation {$R *.fmx} uses [...]GlobalVars; procedure TForm1.LoginBtnClick(Sender: TObject); var Manager : TObjectManager; MyCriteria: TCriteria<TUser>; u : TUser; begin Manager := TObjectManageR.Create(AureliusConnection); MyCriteria :=Manager.Find<TUtilisateur> .Add(TExpression.Eq('login',LoginEdit.Text)) .Add(TExpression.Eq('password',PasswordEdit.Text)); u := MyCriteria.UniqueResult; if u = nil then MessageDlg('Login ou mot de passe incorrect',TMsgDlgType.mtError,[TMsgDlgBtn.mbOK],0) else begin LoggedInUser:=u; //Here I assign my local User data to my global User variable Form1.Destroy; A00Form.visible:=true; end; Manager.Free; end; 

Then, in another form, I would like to access this LoggedInUser object in the Menu1BtnClick procedure:

 Unit C01_Deviations; interface uses System.SysUtils, System.Types, System.UITypes, System.Classes, System.Variants, FMX.Types, FMX.Graphics, FMX.Controls, FMX.Forms, FMX.Dialogs, FMX.StdCtrls, FMX.ListView.Types, FMX.ListView, FMX.Objects, FMX.Layouts, FMX.Edit, FMX.Ani; type TC01Form = class(TForm) [...] Menu1Btn: TButton; [...] procedure Menu1BtnClick(Sender: TObject); private { Déclarations privées } public { Déclarations publiques } end; var C01Form: TC01Form; implementation uses [...]User,GlobalVars; {$R *.fmx} procedure TC01Form.Menu1BtnClick(Sender: TObject); var Assoc : TUtilisateur_FonctionManagement; ValidationOK : Boolean; util : TUser; begin ValidationOK := False; util := GlobalVars.LoggedInUser; // Here i created a local user variable for debug purposes as I thought it would permit me to see the user data. But i get "Inaccessible Value" as its value util.Nom:='test'; for Assoc in util.FonctionManagement do // Here is were my initial " access violation" error occurs begin if Assoc.FonctionManagement.Libelle = 'Reponsable équipe HACCP' then begin ValidationOK := True; break; end; end; [...] end; 

When I debug, I see "Invalid value" in my user value column. Do you know, why?

I tried to put an integer in this GlobalVar module, and I was able to set its value from my login form and read it from my other form.

I think I could store the user id, which is an integer, and then retrieve the user from the database using my id. But it seems really ineffective.

Answer 1

"I assign this data to my global user" - what does it mean?

Through my crystal ball, I see a code similar to this in your login form:

 var user: TUser; begin user := TUser.Create; try // assign properties/fields of user instance LoggedInUser := user; finally user.Free; end; end; 

LoggedInUser points to a freed (and possibly reusable) block of memory. Solving any LoggedInUser property / field is likely to result in an access violation.

Ha, pretty close:

 LoggedInUser:=u; 

Add the Assign(aSource: TUser) method Assign(aSource: TUser) to your TUser class, which performs a deep copy of the values ​​for all fields / properties (rather than link assignments) and calls it instead:

 LoggedInUser.Assign(u); 

Answer 2

To add a little more explanation, consider this section of code:

 begin Manager := TObjectManageR.Create(AureliusConnection); MyCriteria :=Manager.Find<TUtilisateur> .Add(TExpression.Eq('login',LoginEdit.Text)) .Add(TExpression.Eq('password',PasswordEdit.Text)); u := MyCriteria.UniqueResult; 

TUser is now a reference type, so the variable u contains a pointer to an object. In this case, u now points to a UniqueResult object that belongs to your TObjectManager instance.

  if u = nil then MessageDlg('Login ou mot de passe incorrect',TMsgDlgType.mtError,[TMsgDlgBtn.mbOK],0) else begin LoggedInUser:=u; //Here I assign my local User data to my global User variable 

No. Here, you also point LoggedInUser to the same instance of the object that u points to, which is still the UniqueResult your TObjectManager . Also, since your initialization section in GlobalVars already created an instance of TUser (which was indicated by LoggedInUser ), you just rewrote this link and leaked into memory.

  Form1.Destroy; A00Form.visible:=true; end; Manager.Free; end; 

And now you freed up your TObjectManager , destroying it UniqueResult - the object that u and LoggedInUser .

Igor’s proposal to make a deep copy will help fix both of these problems.

  LoggedInUser.Assign(u); 

Firstly, you will not leak memory, since now you assign values ​​to a previously constructed TUser object (and do not rewrite the only link you have)! Secondly, when your TObjectManager freed, you will not destroy the object referenced by LoggedInUser .

If TUser is a custom class, you will need to implement your own deep copy method. See here for an example . You do not need to TPersistent from TPersistent , and you do not need to use Assign , but you need to provide some deep copy function in your TUser class.

As an alternative from the documentation , take a look

OwnsObjects Property

If true (default), all managed objects are destroyed when the TObjectManager is destroyed. If false, the objects remain in memory.

So, before you release your TObjectManager , just do:

 Manager.OwnsObjects := false; Manager.Free; 

And be careful to free up any other objects returned from queries, etc. that you no longer need. This will allow you to assign a link, as you are doing now, and this will prevent the release of TObjectManager when it is destroyed.

Note that this solution still leaves you with a memory leak problem, so you should solve it by checking the instance and freeing it before starting a new assignment or not creating an empty TUser to begin with.

Answer 3

since you are creating an instance of the object in a global variable, which assumes that you want to subsequently assign values ​​to it, rather than assigning a completely new object.

You can do this as suggested using the .Assign method, or you can copy the corresponding fields one by one if there are only a few.

You need a “deep copy” of the fields of the TUser object, and also make sure that if you have any objects inside TUser that you determine whether they should be deeply copied or if the links can be copied instead.

But this is a common mistake that people make. You want one or the other - either create an instance or copy VALUES; or do not create an instance and assign it another instance, for example the one that you get from TObjectManager. But, as also pointed out, the lifetime of the returned TObjectManager cannot be what you expect. (If it uses the interface, it automatically counts and safely assigns to another object. But this can be unobvious, without digging in this case or not.)

Since TObjectManager returns a reference to an object or NIL, you can simply assign the TObjectManager value directly to a global variable. In this case, do not create a default instance in the initialization section.

I also repeat moods around passing an object to form constructors, rather than using a global variable. This allows you to configure unit testing.

I would also like to add a comment that no one has touched yet.

  else begin LoggedInUser:=u; //Here I assign my local User data to my global User variable Form1.Destroy; A00Form.visible:=true; end; 

This is NOT how you destroy form! Because, technically speaking, nothing after calling Form1.Destroy does not work inside a valid instance. It probably works fine because the stack is probably not corrupted; but does anyone know if A00Form.visible will work: = true or not.

And ... this is not how you close the form and still focus on the other. This form should not be aware of any other forms. Again, this is what should be entered when the form is created, if necessary, which is not really the case.

Generally speaking, use the OnClose handler to do what you want when the form closes. But in this case, you don’t even want to.

You want to instantiate the form from another location, and then use ShowModal to display it. When it closes, you want to set up some kind of return status that says if it is registered correctly. If they did, THEN open any next form that you want to see. If not, you probably want to display them again in the login form with the message displayed on it.

It also indicates how you enter a TUser entry into each form - by wrapping the form, it opens inside the method, which creates an instance of the form, passing the TUser object (via the constructor or property), and then gets by after ShowModal returns. This may mean that you only want to HIDE the form at the close, and not for FREE, allowing the control display method to access the form data before destroying the form! (I believe that caHide is the default action for the OnClose handler, so you will need to add OnClose if you want to replace it with caFree. This is because the IDE's default behavior is to automatically create forms in which if you don't want they are freed when they close.If you do not automatically create your forms, you NEED to free them after creating them and displaying them using Show or ShowModal - unless you decide to leave them hanging in memory, which first defeats the goal of bypassing auto-creation. )