String.contains(CharSequence) calls toString on argument

Tomasz Kowalczewski tomasz.kowalczewski at gmail.com
Sat Mar 21 08:21:24 UTC 2015


There are other places which accept CharSequence and iterate over it
not caring about possible concurrent modification (the only sane way
IMHO). Examples are String.contentEquals and String.join that uses
StringJoiner which iterates its argument char by char.

It looks like contains is the exception in this regard.

On 20 mar 2015, at 17:05, Vitaly Davidovich <vitalyd at gmail.com> wrote:

>>
>> If CharSequence is mutable and thread-safe, I would expect toString()
>> implementation to provide the atomic snapshot (e.g. do the
>> synchronization). Therefore, I find Sherman's argument interesting.
>
>
> That's my point about "it ought to be up to the caller" -- they're in a
> position to make this call, but String.contains() is playing defensive
> because it has no clue of the context.
>
> On Fri, Mar 20, 2015 at 11:55 AM, Aleksey Shipilev <
> aleksey.shipilev at oracle.com> wrote:
>
>> If CharSequence is mutable and thread-safe, I would expect toString()
>> implementation to provide the atomic snapshot (e.g. do the
>> synchronization). Therefore, I find Sherman's argument interesting.
>>
>> On the other hand, we don't seem to be having any details/requirements
>> for contains(CharSequence) w.r.t. it's own argument. What if
>> String.contains pulls the values one charAt at a time? The concurrency
>> requirements are not enforceable in CharSequence alone then, unless you
>> call the supposed-to-be-atomic toString() first.
>>
>> -Aleksey.
>>
>>> On 03/20/2015 06:48 PM, Vitaly Davidovich wrote:
>>> Yes, but that ought to be for the caller to decide.  Also, although the
>>> resulting String is immutable, toString() itself may observe mutation.
>>>
>>> On Fri, Mar 20, 2015 at 11:40 AM, Xueming Shen <xueming.shen at oracle.com>
>>> wrote:
>>>
>>>>> On 03/20/2015 02:34 AM, Tomasz Kowalczewski wrote:
>>>>>
>>>>> Hello!
>>>>>
>>>>> Current implementation of String.contains that accepts CharSequence
>> calls
>>>>> toString on it and passes resulting string to indexOf(String). This IMO
>>>>> defeats the purpose of using CharSequences (that is to have a mutable
>>>>> character buffer and not allocate unnecessary objects).
>>>> It is arguable that cs.toString() may serve the purpose of taking a
>>>> snapshot of an otherwise
>>>> "mutable" character buffer?
>>>>
>>>> -Sherman
>>>>
>>>>
>>>> Is changing this a desirable development? It seems pretty
>> straightforward
>>>>> to port indexOf(String) to use CharSequence.
>>>>>
>>>>> If all you need is patch then I can work on it (I have signed OCA) just
>>>>> wanted to make sure it is not a futile work.
>>
>>
>>



More information about the core-libs-dev mailing list