(Mis-)Behaviour of TStringHelper

[Cristian Peța 107]

13 hours ago, FredS said:

Essentially all system wide methods are run through DUnitX now.
This is how I know that RSP-15040 is still in Rio 10.3.1, I use that when anyone asks why I built my own string helper.

You haven't told what exactly is wrong and for me is working as expected.

Sincerely I don't have an hour to lose to identify what's wrong. Specifically what should be expected?

There is only one "Huh?" at "s.LastDelimiter('Hello')" but it returns 53 that is good. What's wrong?

 

P.S. The truth is that Delphi's LastIndexOf is not documented and is working but not as implemented in other languages.

Edited February 19, 2019 by Cristian Peța
P.S.

[FredS 138]

10 hours ago, Tommi Prami said:

That issue is closed, is there duplicate, or why is that? Any idea? Issue states fixed in 10.2 if I understand...

 

10 hours ago, Cristian Peța said:

You haven't told what exactly is wrong and for me is working as expected.

Sincerely I don't have an hour to lose to identify what's wrong. Specifically what should be expected?

There is only one "Huh?" at "s.LastDelimiter('Hello')" but it returns 53 that is good. What's wrong?

 

P.S. The truth is that Delphi's LastIndexOf is not documented and is working but not as implemented in other languages.

 

I didn't want to hijack this thread for something I moved on from years ago, so really quickly:

Berlin Help:  Returns the last index of the Value string in the current 0-based string.

  { TStringHelper.LastIndexOf returns wrong values } 
  //    0                 19                 38                  0-based
  s := 'Hello how are you, Hello how are you, Hello how are you';
  Assert(s.IndexOf('Hello', 38) = s.LastIndexOf('Hello', 38));

As for the "Huh?" next to LastDelimiter, that was me not understanding how that worked.

 

[Cristian Peța 107]

13 hours ago, FredS said:

  { TStringHelper.LastIndexOf returns wrong values } 
  //    0                 19                 38                  0-based
  s := 'Hello how are you, Hello how are you, Hello how are you';
  Assert(s.IndexOf('Hello', 38) = s.LastIndexOf('Hello', 38));

 

Wrong test case.

s.LastIndexOf('Hello', 38) will search starting from 38 to the left. That means in this string 'Hello how are you, Hello how are you, H'.

First occurrences is at 19 so it "Works As Expected".

I know this is not as other implementations but you must specify this in report if you want a change.

And this change can brake old code so it must be strongly justified.

[FredS 138]

10 hours ago, Cristian Peța said:

s.LastIndexOf('Hello', 38) will search starting from 38 to the left. That means in this string 'Hello how are you, Hello how are you, H'.

First occurrences is at 19 so it "Works As Expected". 

 

I moved on in 2016, so what happens with this method is no longer for me only for those who spend hours searching for bugs they can't figure out.
And I'm not looking for the 'First occurrence', that would be 'IndexOf', so either way the function fails.

Returns the last index of the Value string in the current 0-based string.
StartIndex specifies the offset in this 0-based string where the LastIndexOf method begins the search, and Count specifies the end offset where the search ends.
There are six LastIndexOf overloaded methods, each one allowing you to specify various options in order to obtain the last index of the given string in this 0-based string.
When the Value argument (Char or String), passed to LastIndexOf, is not found in the 0-based string, LastIndexOf function returns -1.

 

 

 

 

[Stefan Glienke 2018]

The documentation is not precise enough - but fwiw the implementation as in Delphi is the same as it is in .NET or Java. The startindex actually also limits the length of the searched string. So any search text longer than 1 will only be found at startIndex - (Length(searchText) - 1) or left to that. And when Cristian said "first occurence" he actually meant "first occurence from the right" of course.

Edited February 21, 2019 by Stefan Glienke

[FredS 138]

5 hours ago, Stefan Glienke said:

The documentation is not precise enough

 

True but it is the same as IndexOf, should I now file a bug report for IndexOf?

 

StartIndex specifies the initial offset in this 0-based string where the search starts

Because this does not compute:

Assert(s.IndexOf('Hello', 38) = s.LastIndexOf('Hello', 38));

I originally looked at the source and also took " startIndex - (Length(searchText) - 1)" into account but here is that math:

  //    0                 19                 38                  0-based
  s := 'Hello how are you, Hello how are you, Hello how are you';
  //                                    ^ 32                     Startindex - (Length('Hello')- 1), searched forward or backward?

IMO at this point the claim that it works as expected needs to come with new documentation or a bug fix.

 

[Stefan Glienke 2018]

Sorry I meant

startIndex + Length(searchText) - 1

because the method simply works as follows: it starts to search at startIndex but does not consider any character in the string after that. That causes it not finding any searched text that extends beyond that index.

So as Christian already stated it basically works as if you cut the string after startIndex.

 

So what the documentation is simply missing is the fact that it would not find any occurence that extends past startIndex when you use any of those overloads, that's all.

Edited February 21, 2019 by Stefan Glienke

[FredS 138]

 

13 minutes ago, Stefan Glienke said:

it basically works as if you cut the string after startIndex

 

Yeah, certainly made clear in the documentation </sarcasm>

One would hope that when they copied a DotNet function they could at least copy the DotNet explanation:

The search starts at a specified character position and proceeds backward toward the beginning of the string for the specified number of character positions.

At this point the claim that it works as expected needs to come with new documentation.

[Attila Kovacs 631]

It's a bullsh@t. The doc is a big ~, the mimicked java/.net implementation is a ~, and the parameter name "startIndex" is a ~. It should be called lastIndex as the function name reflects only an index lookup, and not any search in some direction, and mostly not the underlying implementation.

Edited February 21, 2019 by Attila Kovacs

[FredS 138]

2 hours ago, Attila Kovacs said:

should be called lastIndex

LastIndex and DownRange would help but honestly, this is like having to flip your steering wheel when you shift the car into reverse. 🙂