[fpc-devel] Questions on TStringList.Find change (Mantis 28744)

[David Jenkins]

On 3/22/16 10:48 AM, Michael Van Canneyt wrote:
>
>
> On Tue, 22 Mar 2016, David Jenkins wrote:
>
>> No one here holds Delphi up on a pedestal - least of all me.  My 
>> viewpoint is completely practical/funcitonal - I have to make our 
>> code base work with FPC/LCL and VCL.  Given the way we use find 
>> functionality (with a mixture of naturally sorted and automatically 
>> sorted lists) it has ceased to work with both (it did previously). I 
>> want to fix that as elegantly and as effectively as possible. Making 
>> suggestions for FPC improvement is a side effect - and as we both 
>> agree TStringList.Find can use some improvement.
>
> "by intention not by accident." Clearly this is not the case, given 
> the warning in Delphi's help.
I do not see it as a warning.  It is a statement of requirement on the 
input.  By passing in a naturally sorted list I am complying with that 
requirement (the requirement is only that it be sorted not how it is 
sorted).  So the code works not by accident but because I have complied 
with the requirement and the code functions properly.   Adding a check 
to make sure I comply with the requirement makes the code more robust 
and is a great thing to do.  Checking for 'Sorted' actually doesn't 
always do this.

The problem is that the Sorted flag is both a request and a loose 
statement of state.  My list can be sorted without me requesting 
(through the flag) that the code sort and maintain the list.  But 
throughout the code (Delphi and FPC) 'Sorted' is treated as an accurate 
mirror of list state whether the list is automatically sorted, naturally 
sorted, or not sorted at all (it can't tell the difference between the 
latter two).  In the case of naturally sorted this state can be 
inaccurate (Sorted not set) and so some code won't run or accurate 
(Sorted set) but requesting unneeded actions.

To me, after spending this amount of time thinking about it, this calls 
out for improvement as much as the concern about an accidental call to 
Find with an unsorted list.  Though I understand that getting an 
erroneous result, unknowingly, is far more critical than inefficient 
code.   However, for us we are parsing a whole lot of text strings - 
running through unneeded calls to sort (no matter how fast it might be 
on an already sorted list) adds up.  And not running at all is a failure 
of function.
> In each case, there is no argument that the code can be improved.
>
> We'll sort something out in the end.
>
> Is your preference for an extra argument to Find() or an additional 
> property ?
>
> Michael.
> _______________________________________________
> fpc-devel maillist  -  fpc-devel at lists.freepascal.org
> http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

I do not have a preference.

I like the auto param because 1) it is simple 2) doesn't change the 
interface for code that doesn't care and 3) solves the base problem.  
However, it doesn't decouple "Sorted" request from sorted state.

An additional property could solve the base problem and clarify what the 
list state actually is.  Though additional code would need to change to 
take advantage of the clarity.

Maybe something (not extensibly thought on) like

Sorted: boolean read GetSorted write SetSorted
NaturalSorted: boolean read FNaturalSorted write FNaturalSorted

SetSorted(AValue: Boolean)
begin
FAutoSorted := AValue;  // This is the request to auto sort or not
end

GetSorted: Boolean
begin
Result := FAutoSorted or FNaturalSorted  // This gets the state right 
for both Auto and Natural (as long is Natural truly is :D)
end

or have a ListType property that is ltAutoSort, ltNaturalSort, ltNone

and GetSorted = ListType in [ltAutoSort, ltNaturalSort]


The additional properties (not necessarily implemented as above) is 
attractive as it seems like a more complete solution.  There is still 
the chance of an unsorted list getting by.

Adding Denis' solution in .Find along with the new properties would 
narrow the window of unsorted lists coming through while still allowing 
bypass of sorting code for explicitly set Naturally sorted lists (lists 
with NaturallySorted=True but not actually sorted could still sneak 
through).

All the above are fine with me.

David