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

[David Jenkins]

On 3/21/16 3:44 PM, Michael Van Canneyt wrote:
>
> The reason for this behaviour is that find needs to be sure that the 
> list is sorted.
> If it thinks the list is not sorted, the result cannot be guaranteed 
> to be
> correct, and this is an error condition.
>
> What I don't understand about these discussions is:
>
> TStringList is just one possible implementation of TStrings. It is 
> only there for your ease of use.
>
> If you don't like it, you can use another one. You can find several. 
> There is one in inifiles, the LCL probably also contains a couple.
>
> If all APIs work with TStrings, then they don't care what TStrings
> descendent you use. So you are free to use one that does whatever you 
> want.
>
> Anyone declaring an API that explicitly expects TStringList, is simply 
> doing something wrong, in my view.
>
> This is Object Pascal for a reason, after all.
>
> Michael.
> _______________________________________________
> fpc-devel maillist  -  fpc-devel at lists.freepascal.org
> http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel

I understand the need for the list to be sorted and agree that calling 
find on an unsorted list is an error condition.

I am not convinced that current change deals with that well.

For one, the Sorted flag and actual state of the list are not directly 
linked.  I can manually sort the list so that Find binary search will 
work just fine.  But Find will exit before the binary search can occur - 
without any indication why.

Ff the list is not sorted,  I don't think the current change creates a 
true error response to the error condition.  Returning False indicates 
string not found - not an error.  Furthermore I think the change makes 
the error condition worse by leaving values in Index that can cause 
crashes beyond just the error condition of not finding the right string.

Raising an exception if not Sorted would be an error response.  Or 
providing some feedback to the user to indicate the difference between 
not found and error condition.

We can use our own descendant of TStrings or we could apply a local 
patch to TStringList as already mentioned.  So we can actually create a 
work around.  Our interest was in pointing out a change in fpc code that 
we thought was a regression rather than an improvement.