Should String substringFrom:to: check indexes?

[ltratt]

At the moment substringFrom:to: is implemented thus:

    substringFrom: start to: end = (
        ((end <= self length) && (start > 0) && (start <= end))
            ifTrue: [^self primSubstringFrom: start to: end]
            ifFalse: [
                self error: 'Attempting to index string out of its bounds (start: ' + start asString + ' end: ' + end asString + ' length: ' + self length asString + ')' ]
    )

which feels unidiomatic because:

1. there's no way that primSubStringFrom:to: can not recheck those indexes, since any old Tom, Dick, or Harry can call primSubStringFrom:to: with whatever arguments they want.
2. other indexing primitives such as Array at: do not have a primAt variant (i.e. the VM is expected to check the indexes).

I was expecting this part of the library to just be substringFrom:to: = primitive -- perhaps we can simplify (and speed up!) this code by doing so?

[smarr]

The people implementing it may have had different trade offs in mind.

https://github.com/SOM-st/CSOM/blame/8c154cd32590d380c366ec2d834404cd7045bfdf/src/primitives/String.c#L93-L111

https://github.com/SOM-st/SOMpp/blame/e474d1d75ac2ef8a361f43378bf5115bd4aa66c2/src/primitives/String.cpp#L114-L127

SOM is suppose to be simple. Simple comes in some places with compromises on "doing the perfect thing", when it complicates matters. As you have seen already in multiple cases. It might also be the Smalltalk way of doing things... don't know.

Though, I am happy to have those things fixed up step by step.

[ltratt]

Though, I am happy to have those things fixed up step by step.

Do you have a suggestion for what you might like to see?

[smarr]

First step would be to have the same semantics as the method, simply implemented in the primitive, I suppose.

Not sure what else there would be to do.
If you'd want an error handler, one could define a String>>#indexOutOfBounds:, which could be triggered instead of the error:.

Though, and you'll love that: the semantics around which classes can be subclassed aren't exactly consistent in the various implementations. So, the handler may not be very useful in practice.

[ltratt]

I guess my question is slightly different. There are lots of SOMs: can one change the library without changing all of them? I think the answer is "no" as you recently added primitives that I think only Java SOM implements? But maybe I'm wrong!

[smarr]

All SOMs I am maintaining link to a specific version of a library, so, there's no direct negative impact of changing the library.

However, the library is tested with some of those SOMs (https://travis-ci.org/github/SOM-st/SOM), and I am generally aiming to have them all support the latest version.

And, this is work, yes...

[ltratt]

Understood. I'm definitely not in the situation (for various different definitions of "situation") where I want to take the burden of evolving any SOM except yksom, so I shall shut up on this topic!

[smarr]

Nah, don't. Especially, don't feel obliged to actually provide fixes.
The first step of fixing things is having them documented/reported...

[ltratt]

Point taken. I will do my best to try not to raise things which are partly matters of taste though.