TStringList can do everything a little, but nothing wellWhat are you using TStringList for?
So it is always a bad idea to use TStringList anywhere
TStringList can do everything a little, but nothing well
So it is always a bad idea to use TStringList anywhere
What class(es) would you suggest as an improvement on TStringList, and how are they better?Any generic map or dictionary? Because they are type safe for all elements. But you know all that....
I found "AddObject", "InsertObject", but I didn't find "DeleteObject". Do you have any idea for deleting an object ? Maybe "Reduce" ????
I found "AddObject", "InsertObject", but I didn't find "DeleteObject".
I found "AddObject", "InsertObject", but I didn't find "DeleteObject". Do you have any idea for deleting an object ? Maybe "Reduce" ????
Not tested.....
MyList.Objects[SomeValidIndex].Free; //FreeAndNil(MyList.Objects[SomeValidIndex]);
QuoteI found "AddObject", "InsertObject", but I didn't find "DeleteObject".
If you just want to remove it from the list:
mylist.Objects[i] := nil;
Just in case this isn't clear: this will not free the object instance.
What class(es) would you suggest as an improvement on TStringList, and how are they better?
A common problem with descendants of classes like TFPGMap (TFPSList) is, that they have one or more public constructors and/or methods you really don't want any interfacing code to use. (Anything that handles plain pointers, for a start.) Not a problem if you're the sole developer, but not so good in a team or interface. And writing your own custom container class from scratch every time gets old fast.What class(es) would you suggest as an improvement on TStringList, and how are they better?Any generic map or dictionary? Because they are type safe for all elements. But you know all that....
FreeAndNil(MyList.Objects[SomeValidIndex]); gives a compile error ;O(Why FreeAndNil at all? With setting a pointer variable to nil you signal to other pieces of your code that this object does not exist any more. But this is not necessary here at all since you free the entire list and thus it is impossible to access the individual pointers to these objects any more. Do not confuse the pointers stored in the list with other variables pointing to the same object instance - they will not be nil'ed anyway even if you call FreeAndNil for MyList.Objects[..].
What class(es) would you suggest as an improvement on TStringList, and how are they better?
For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end . And TYourObject should be an object or record, not a class.
But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.
FreeAndNil(MyList.Objects[SomeValidIndex]); gives a compile error ;O(Why FreeAndNil at all? With setting a pointer variable to nil you signal to other pieces of your code that this object does not exist any more. But this is not necessary here at all since you free the entire list and thus it is impossible to access the individual pointers to these objects any more. Do not confuse the pointers stored in the list with other variables pointing to the same object instance - they will not be nil'ed anyway even if you call FreeAndNil for MyList.Objects[..].
Simply .Free the object:
MyList.Objects[SomeValidIndex].Free;
Handoko, you have right with your point:
FreeAndNil(MyList.Objects[SomeValidIndex]); gives a compile error: Can't take the address of constant expressions.
MyObject := TMyClass(MyList.Objects[SomeValidIndex]);
FreeAndNil(MyObject); gives no error
For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end . And TYourObject should be an object or record, not a class.
A common problem with descendants of classes like TFPGMap (TFPSList) is, that they have one or more public constructors and/or methods you really don't want any interfacing code to use. (Anything that handles plain pointers, for a start.) Not a problem if you're the sole developer, but not so good in a team or interface. And writing your own custom container class from scratch every time gets old fast.What class(es) would you suggest as an improvement on TStringList, and how are they better?Any generic map or dictionary? Because they are type safe for all elements. But you know all that....
But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.
Most of the time because the vars just go out of scope, joining the quire invisible. But if they don't you'd better nil 'em.
I'm afraid that I'm very sceptical that this is good advice for a newcomer.For a list of strings and objects, I would suggest array of record a: string; b: TYourObject; end . And TYourObject should be an object or record, not a class.
OP has already said that he's using a TStringList, and is asking for advice in that context.
Muddying the water by introducing the obsolete Turbo Pascal object (if that is your intention), or by overlooking the fact that OP already knows that he needs an /instance/ of a class rather than the class itself, is not helpful.
MarkMLl
Indeed. I would even go as far to advise only making and using classes, simply to keep it all simple and consistent. Learn to do one thing well.
But as @wp said, maybe you don't need to use FreeAndNil. I'm not interested into the debate of using FreeAndNil or not, but I only used it once in a program I wrote, and I really had the reason why FreeAndNil was needed. For all the other programs I wrote, using Free only, seem to run without any issue. So, I can sure to say 99% Free only is enough.
Most of the time because the vars just go out of scope, joining the quire invisible. But if they don't you'd better nil 'em.
This is only true for managed types and record and object instances that are located on the stack (or are global variables). Class instances and any other manual allocations need to be freed explicitely as well.
Btw, FreeAndNil is sometimes needed, because Free leaves a dangling pointer,That is the basic mistake.
It is bad programming practice to call Destroy directly. It is better to call the Free (https://www.freepascal.org/docs-html/current/rtl/system/tobject.free.html) method, because that one will check first if Self is different from Nil.
To clean up an instance and reset the reference to the instance, it is best to use the FreeAndNil (https://www.freepascal.org/docs-html/current/rtl/sysutils/freeandnil.html) function
But can anyone please provide real demos showing FreeAndNil is really needed? And if we change that FreeAndNil then the program will cause error.I normally stay away from discussions about OOP but, in this particular case, it's not just about OOP.
FreeAndNil or not have been discussed for years but still have no a mutual agreement.
But if till then still no one is able to provide a real demo then I will keep believing 99% Free only is enough.
It is your choice and your preferences.
As far as compile-able code, the subject "FreeAndNil" in the FPC Wiki, has an example that shows the problem with just Free. It's also worth noting that there is code out there that tests if some class is Assigned before it proceeds to do something with it. If the class/pointer is freed and not nil-ed, Assigned will return TRUE causing the rest of the code to access memory that is completely unrelated to the tested class.
Yes, that was I what did. And my code won't run correctly without using FreeAndNil. I didn't write the code to proof FreeAndNil, I really 'accidentally' encountered such case. But only once so far. So I later introspected, I thought maybe the problem was the code design.Please teet the following scenario, but first make sure that Form2 is not in the Auto-Create Forms:
I don't mean we shouldn't use FreeAndNil. I just want to proof 99% Free only is enough. Because the cases that FreeAndNil is required are really rare.
I don't mean we shouldn't use FreeAndNil. I just want to proof 99% Free only is enough. Because the cases that FreeAndNil is required are really rare.It's important to realize that it's not a matter of how rare something is needed. It's simply good programming and that should be _continuous_.
The "clean" (for lack of a better word) way is to use FreeAndNil. The reason is very simple, a class is a pointer to a heap memory block. Once the memory block has been freed, it should not be accessed again. If the class/pointer has not been nil-ed then it is possible to dereference the pointer after the memory has been freed, which is a programming error but, it often won't be visible immediately. if the pointer is nil-ed then any subsequent de-reference will cause an access violation (as it should) revealing the programming error right then and there.
I don't condemn FreeAndNil, but I try to avoid it as much as possible because it can hide bugs. Everybody thinks exceptions are a bad thing - no they are here to help you to find bugs. Checking freed pointers for nil is a habit which prevents exceptions - but along with automatic nilling deallocated pointers this prevents you from seeing the bug.On the contrary, if the pointer is nil and it's dereferenced, there will be an exception (access violation.) Thus clearly revealing the presence of a bug.
So, my message is: Never type FreeAndNil without thinking. I agree with Handoko above that calling .Free instead of FreeAndNil is better in most cases.
But by modifying your example you can show that FreeAndNil is useful:It is always difficult it create useful and non-trivial examples... But as I wrote the example is constructed such that the Button1Click contains a bug, namely destroying the list immediately after loading the file which should be done somewhere else (certainly in FormDestroy or in some other button click). Assuming that it is a bug, nilling the instance by FreeAndNil makes it even worse because now the bug is hidden and hard to find, rather than raising an easily observed exception.
procedure TForm1.Button1Click(Sender: TObject); begin FList := TStringList.Create; FList.LoadFromFile('data.txt'); FList.Free; end;
procedure TForm1.Button2Click(Sender: TObject); begin DoSomethingElse; if FList <> nil then // Do we still have a valid reference to the class? Nobody knows... FindDuplicateLines(FList); end;
For instance, declaring the initial value of a variable in a function is actually rarely needed because the common case is that the variables will be assigned some value in the code (at least they should) but, by initializing them in the declaration you know what their initial value is, instead of being whatever they overlay on the stack, because of that, if something isn't working quite right, it will usually be much easier to find and also, the behavior will almost always be reproducible because the variable are always initialized to a specific value. Whereas if one or more variables are left uninitialized, they'll have whatever values happened to be on the stack causing the behavior to often vary, making the bug harder to find.
So, my message is: Never type FreeAndNil without thinking.
I remember some said misusing FreeAndNil may make the code harder to debug because it hides the real issue ... or maybe something like that. Can anyone explain in more detail?
But I am more interested to see how frequent or rare is really needed.That will vary from program to program depending on how many blocks the program dynamically allocates, frees and re-uses.
I remember some said misusing FreeAndNil may make the code harder to debug because it hides the real issue ... or maybe something like that. Can anyone explain in more detail?I am of the opposite opinion. FreeAndNil should always make the code easier to debug because, if a pointer is nil then the reason why the de-reference is failing is completely obvious, whereas if the pointer has a value that points somewhere in memory, it will be much more difficult to figure out that it shouldn't be pointing there and figuring out what the values mean will be an exercise in futility because the pointer has no business pointing there.
Thank you for providing those codes. But I think I need to rephrase what I said. The codes you showed are specially written to show bad programming practice. I would say those were the programmer fault. Why do still use that variable when you know for sure you have free it?I agree that you can consider my example a bad programming practice, but I had to come up with something simple to illustrate the issue. In other, not so trivial cases it's very useful to set a particular object to nil.
I remember some said misusing FreeAndNil may make the code harder to debug because it hides the real issue ... or maybe something like that. Can anyone explain in more detail?Did you see my example in reply #46?
There is another button which is supposed to analyze the contents of the stringlist, maybe for duplicate lines. Because for some reasons the click handler of this button performs some other things the button can be clicked before the FList has been created, and you decide to simply check FList for not being nil:
MyList.Objects[SomeValidIndex].Free;
What the difference between "Free" and "FreeInstance" ? Does it suppress the allocated memory for Objects ?
FreeAndNil is recommended in the FPC documentation here (https://www.freepascal.org/docs-html/current/rtl/system/tobject.destroy.html):QuoteIt is bad programming practice to call Destroy directly. It is better to call the Free (https://www.freepascal.org/docs-html/current/rtl/system/tobject.free.html) method, because that one will check first if Self is different from Nil.
To clean up an instance and reset the reference to the instance, it is best to use the FreeAndNil (https://www.freepascal.org/docs-html/current/rtl/sysutils/freeandnil.html) function
The "clean" (for lack of a better word) way is to use FreeAndNil. The reason is very simple, a class is a pointer to a heap memory block. Once the memory block has been freed, it should not be accessed again. If the class/pointer has not been nil-ed then it is possible to dereference the pointer after the memory has been freed, which is a programming error but, it often won't be visible immediately. if the pointer is nil-ed then any subsequent de-reference will cause an access violation (as it should) revealing the programming error right then and there.
I'd be happier if it were documented as being indivisible.
Since it's not, there's the potential for an avoidable race condition when the pointer is first inspected.
I don't condemn FreeAndNil, but I try to avoid it as much as possible because it can hide bugs. Everybody thinks exceptions are a bad thing - no they are here to help you to find bugs. Checking freed pointers for nil is a habit which prevents exceptions - but along with automatic nilling deallocated pointers this prevents you from seeing the bug.
PS D:\fpc\git> .\testoutput\treuse.exe
01595A00 0000002A
01595A00 00050001
It is interesting, that most languages with garbage collection assume that object instances exist until they go out of scope everywhere. So, what are you supposed to do if you want to replace that instance with another one? I do that often.
I mean, are you going to call Destroy and Create a new one with the same pointer? How do you make sure the other pointers don't keep pointing to the depreciated instance?
A TStringlist with objects is a MAP/DICTIONARY pattern as I wrote. So use a map/dictionary. Not the legacy code. That is core.
I'd be happier if it were documented as being indivisible.
Since it's not, there's the potential for an avoidable race condition when the pointer is first inspected.
Documenting it as indivisible would not change that FreeAndNil simply is not indivisible. If you access the to be freed instance from multiple threads then you need to use synchronization.
There are two anti patterns here:
1. Misusing a stringlist to store objects (a relic from the past)
2. The Free and Nil anti pattern.
The point is that as things stand the documentation says nothing either way, and I suspect that some are assuming that it is safer than it really is.
In general it should be assumed that the RTL isn't threadsafe. The only exception are the internal management operations for strings, arrays and such. And types like TThreadList.
Var MyClass : TMyClass; //Or TObject Begin MyClass:=TMyClass(MyList.Objects[SomeValidIndex]); //If it's TObject above, no need for Casting End;
The corresponding object ofis
mylist.Strings[i]
mylist.Objects[i]
It returns a TObject, so if you want to access the class you initially put there you have to use a typecast as Zvoni already mentioned. And as you wrote that only some lines are associated with an object, you'd better test like this:or, even better:
if Assigned(myList.Objects[i]) then // equivalent to: if (myList.Objects[i] <> nil) then myclass := TMyClass(myList.Objects[i]);The 'is'-test tests for nil as well. And you know for sure that it is actually of the class you want and expect.
if (myList.Objects[i] is TMyClass) then myclass := TMyClass(myList.Objects[i]);
Works
... s:=IntToStr(Integer(MyClass)); //Or use TypeHelper "ToString" --> s:=Integer(MyClass).ToString; ...
No Idea what you're doing wrong. I tested my code above and it works.
Works
... s:=IntToStr(Integer(MyClass)); //Or use TypeHelper "ToString" --> s:=Integer(MyClass).ToString; ...
Something is wrong, somewhere...:(
s:=IntToStr(Integer(MyClass)); // compiler says : Error: Illegal type conversion: "TObject" to "LongInt" // with the word Integer underlined with red //Or use TypeHelper "ToString" --> s:=Integer(MyClass).ToString; s:=Integer(MyClass).ToS // shows ONLY "ToSingle"
No Idea what you're doing wrong. I tested my code above and it works.Maybe it's a WidgetSet problem...
To get the Typehelpers you have to include SysUtils in your Uses-ClauseAlways present :
Something is wrong, somewhere...The size of the Integer type does not always match the size of a pointer. Use the SizeInt type instead.:(
s:=IntToStr(Integer(MyClass)); // compiler says : Error: Illegal type conversion: "TObject" to "LongInt" // with the word Integer underlined with red //Or use TypeHelper "ToString" --> s:=Integer(MyClass).ToString; s:=Integer(MyClass).ToS // shows ONLY "ToSingle"
Use the SizeInt type instead.