Recent

Author Topic: Problems with StringLists  (Read 4596 times)

apexcol

  • Jr. Member
  • **
  • Posts: 54
Problems with StringLists
« on: August 11, 2017, 12:24:15 pm »
I have problems with TStringList...

Code: Pascal  [Select][+][-]
  1. procedure TotaliseByNames;
  2. var
  3.         SL : TStringList;
  4.         I1, I3 : integer;
  5.         vElement: TDOMElement;
  6.         vInputList: TDOMNodeList;
  7. begin   // vInputList is out of scope but is another TStringList
  8.         SL := TStringList.Create;
  9.         SL.Sorted := true;
  10.         for I1 := 0 to pred(vInputList.Count) do begin
  11.                 vElement := vInputList[I1];
  12.                 SL.Find(vElement['name'], I3);
  13.                 if I3 = -1 then  // does not exist then
  14.                         SL.AddObject(vElement['name'], vElement) // create the first
  15.                 else begin
  16.                         {some other code that modifies vElement with aritmethic operations...}
  17.                 end;
  18.         FAnotherList.Assign(SL);               
  19. end;
  20.  

on the Finding it breaks the program...

Also I tried with IndexOf and it shows a number like 8261232 instead of -1,
I checked the code inside and it has some things to fix, for example
in stringl.inc line 936 instead of Result:=Result+1; it must be inc(Result), and
on sorted list, the add or insert method ought to add it on the right place.

rvk

  • Hero Member
  • *****
  • Posts: 6112
Re: Problems with StringLists
« Reply #1 on: August 11, 2017, 01:59:04 pm »
Inc(Result) is the same as Result:=Result+1;

Furthermore... if IndexOf() doesn't work correctly then your Add() or AddObject() also doesn't work correctly for sorted lists.

Next, your code is flawed (or at least your "copy-paste" is).
You have FAnotherList.Assign(SL); inside your for I1 loop. I can't imagine that's the way you intended it (but it was probably a copy-paste error).

Maybe you can create a small compilable example.
For me, your code stops at vElement := vInputList[I1] with incompatible types.

What happens if you first put vElement['name'] into a stringvariable and use that in AddObject() ?

also... you need to use the contsruct:
Code: Pascal  [Select][+][-]
  1. if SL.Find(vElement['name'], I3) then
  2.   // do something with I3.
  3.  
You can't count on I3 being a valid result if the result of Find() is false (boolean).
(reason is to be found in TStringList.Find. If you see, the Index is set to L at the end of the function but the L is only valid if the Result is set to True. I expect it will be false for your 8261232 value)

So my guess is that passing vElement['name'] directly doesn't pass a valid stringvariable to the Find function. You can test that easily by putting it in a stringvariable first.
« Last Edit: August 11, 2017, 02:05:29 pm by rvk »

edgarrod71

  • Jr. Member
  • **
  • Posts: 68
Re: Problems with StringLists
« Reply #2 on: August 12, 2017, 01:16:26 am »
First, when the compiler translates to machine code, Inc is faster than adding...

Result := Result + 1;  yields the following assembly code:

mov  ax,  [bp-02]
inc  ax
mov  [bp-02], ax

Now, inc(i) does it this way:

inc  word ptr [bp-02]

Furthermore... when the string parameter enters into the IndexOf, there is no checking if it has elements or not... so it starts to compare to the pointer and not with the elements of the StringList.

and sorry, my code is not copy-paste...

the FAnotherList.Assign(SL) is intended to take the grouped list out of scope.

in the vElement := vInputList[I1] you're right, I forgot to write TDOMElement(
so it's like

Code: Pascal  [Select][+][-]
  1. vElement := TDOMElement(vInputList[I1]);
  2.  
and I tried also with a string variable put from vElement['name'];

but you know? you got a point on the construct...  thanks, but I need the IndexOf result when it is positive, because if it's positive value it means it found the string, but it goes and goes... to the total capacity of stringlist that is 8261232 instead of -1.

for a faster run, I suggest on the stringl.inc like this...

Code: Pascal  [Select][+][-]
  1. function TStringList.Find(const S: string; out Index: Integer): Boolean;
  2.  
  3. var
  4.   L, R, I: Integer;
  5.   CompareRes: PtrInt;
  6. begin
  7.   Result := false;
  8.   Index:=-1;
  9.   if Not Sorted then
  10.     Raise EListError.Create(SErrFindNeedsSortedList);
  11.   // Use binary search.
  12.   L := 0;
  13.   R := Pred(Count);  // Count - 1;
  14.   if L<=R then  //  while (L<=R) do
  15.   repeat    // repeat is 5-10% faster than while, so
  16.     I := L + (R - L) shr 1; // div 2;  shr is faster than div...
  17.     CompareRes := DoCompareText(S, Flist^[I].FString);
  18.     if (CompareRes>0) then
  19.       L := Succ(I); // I+1;  instead of adding, Succ or Pred only checks... so faster code.
  20.     else begin
  21.       R := Pred(I); // I-1;
  22.       if (CompareRes=0) then begin
  23.          Result := true;
  24.          if (Duplicates<>dupAccept) then
  25.             L := I; // forces end of while loop
  26.       end;
  27.     end;
  28.   until L>R;
  29.   Index := L;
  30. end;
  31.  
« Last Edit: August 12, 2017, 01:24:00 am by edgarrod71 »

edgarrod71

  • Jr. Member
  • **
  • Posts: 68
Re: Problems with StringLists
« Reply #3 on: August 12, 2017, 05:34:41 am »
I found that there must be a simple correction in stringl.inc file...
simply add (count=0) or to the line having Not Find(S,Result) then

Code: Pascal  [Select][+][-]
  1. function TStringList.IndexOf(const S: string): Integer;
  2.  
  3. begin
  4.   If Not Sorted then
  5.     Result:=Inherited indexOf(S)
  6.   else
  7.     // faster using binary search...
  8.     If (count = 0) or Not Find (S,Result) then
  9.       Result:=-1;
  10. end;
  11.  

rvk

  • Hero Member
  • *****
  • Posts: 6112
Re: Problems with StringLists
« Reply #4 on: August 12, 2017, 06:23:55 am »
First, when the compiler translates to machine code, Inc is faster than adding...
Ok, I thought this might have been optimized by the compiler but apparently it's not.
But you might have mentioned it was more optimized. Not that it "must be".
(although I'm not sure if with optimization the code might be different)

Quote
the FAnotherList.Assign(SL) is intended to take the grouped list out of scope.
Look again closely at your code. The Assign(SL) is INSIDE the for loop. You indented the end wrong. So if you worried about optimization you might want to place that FAnotherList.Assign() OUTSIDE the for loop. (That's why I thought that was a copy-paste error)

I found that there must be a simple correction in stringl.inc file...
simply add (count=0) or to the line having Not Find(S,Result) then
No, that's not a MUST !! It might be a small optimization, though.
But if you look in Find() you'll see, that if count=0, it falls through the L<=R line and Result is false. So (count=0) is the same as Not Find(). So you don't have to check them both.

You still didn't mention if you tried to check the result of Find().
I think you might not be understanding the Find() function correctly.

Note the wiki:
Quote
If the string is not found, the function will return False and Index will contain the position where the string will be inserted if it is added to the list.
So, if the string is NOT found, the Index will NOT be -1 (!!!), but it will be the position it would be inserted (hence the 8261232). The function-result will be false in that case. So don't just assume when the result = -1 that the string is not found. You NEED to check the result of Find().

So you need to do something like this:

Code: Pascal  [Select][+][-]
  1.     if SL.Find(vElement['name'], I3) then
  2.     begin
  3.        // I3 is the found string-index
  4.       {some other code that modifies vElement with aritmethic operations...}
  5.     end
  6.     else
  7.     begin
  8.       // you could use I3 to insert at position but Add() will do this too (again, so less optimized)
  9.       SL.AddObject(vElement['name'], vElement) // create the first
  10.     end;
  11.  
« Last Edit: August 12, 2017, 06:29:16 am by rvk »

edgarrod71

  • Jr. Member
  • **
  • Posts: 68
Re: Problems with StringLists
« Reply #5 on: August 12, 2017, 08:23:16 am »
Quote
Ok, I thought this might have been optimized by the compiler but apparently it's not.
But you might have mentioned it was more optimized. Not that it "must be".
(although I'm not sure if with optimization the code might be different)

multiply 3 lines by the number of items to be added simultaneously...  I think big numbers make the difference... or not?

Quote
No, that's not a MUST !! It might be a small optimization, though.
But if you look in Find() you'll see, that if count=0, it falls through the L<=R line and Result is false. So (count=0) is the same as Not Find(). So you don't have to check them both.

You still didn't mention if you tried to check the result of Find().
I think you might not be understanding the Find() function correctly.

in the if with an OR, if the compiler finds the first argument to be true, it does not check the second one... that's a Pascal rule, so I insist..: All said to confirm that this implementation of StringLists needs to be fixed...

By the way, instead of getting an answer, I feel that I get some kind of violence here... relax, ok?

Thaddy

  • Hero Member
  • *****
  • Posts: 14213
  • Probably until I exterminate Putin.
Re: Problems with StringLists
« Reply #6 on: August 12, 2017, 08:40:12 am »
in the if with an OR, if the compiler finds the first argument to be true, it does not check the second one... that's a Pascal rule, so I insist..: All said to confirm that this implementation of StringLists needs to be fixed...

No, it is not a Pascal rule, it is a compiler optimization: Complete Boolean Evaluation, which can be either on or off. This optimization has side effects....
https://www.freepascal.org/docs-html/current/prog/progsu4.html#x11-100001.2.4

If I were you I'd better read what RvK wrote again: he happens to be correct.....in every detail....
« Last Edit: August 12, 2017, 08:44:50 am by Thaddy »
Specialize a type, not a var.

edgarrod71

  • Jr. Member
  • **
  • Posts: 68
Re: Problems with StringLists
« Reply #7 on: August 12, 2017, 08:58:45 am »
But I'm not you, thanks for the trying of help...

rvk

  • Hero Member
  • *****
  • Posts: 6112
Re: Problems with StringLists
« Reply #8 on: August 12, 2017, 09:28:59 am »
By the way, instead of getting an answer, I feel that I get some kind of violence here... relax, ok?
Sorry you feel that way... but I did give a (possible) solution in my answer. I wasn't trying to be violent. I tried to give an answer (see the last part of my last answer). Did you try to use the result of Find() (false/true) instead of just checking -1. Does that work?

Also... the "(count=0) or" addition is not a must but optimization. The "not Find()" will already return true in case count is 0. Checking true or true isn't needed. So the Result := -1 will still be set. So it's an optimization ("(count=0)" is faster than "not Find()"), but it's not a must. It's not a bug. But if you feel this optimization should be part of the code you could create a bugreport so it can be fixed. This is also the case for the Inc(Result) optimization. My comments about this was not an attack but I was under the impression you thought that this was a bug in the FPC-code (you said it must be fixed). If that impression was wrong, I'm sorry.

« Last Edit: August 12, 2017, 09:33:20 am by rvk »

edgarrod71

  • Jr. Member
  • **
  • Posts: 68
Re: Problems with StringLists
« Reply #9 on: August 12, 2017, 11:18:04 am »
You are a gentleman... now I'm happy, and I also apologize if I offended...

What I want to do is to group a StringList items to check their lengths and keep the longest string inside.  I don't know exactly how many items it will take, and I know there are so many repeated values, so that's why I want to group them to process it later on another function.

 

TinyPortal © 2005-2018