Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Add charAt method to both Str and Stringable #46349

Merged
merged 3 commits into from
Mar 5, 2023
Merged

[10.x] Add charAt method to both Str and Stringable #46349

merged 3 commits into from
Mar 5, 2023

Conversation

localusercamp
Copy link
Contributor

What does this PR propose

This PR adds new charAt method to both Illuminate\Support\Str and Illuminate\Support\Stringable.
charAt method allows to get a character by index from a multibyte string.

If this PR is accepted, you can do the following:

str('Hello, world!')->charAt(1); // (string) 'e'
str('Привет, мир!')->charAt(0); // (string) 'П'
str('「こんにちは世界」')->charAt(4); // (string) 'ち'

It also supports negative indexes:

str('Hello, world!')->charAt(-1); // (string) '!'
str('Привет, мир!')->charAt(-12); // (string) 'П'
str('「こんにちは世界」')->charAt(-2); // (string) '界'

If index is greater then string length, or if negative index less then negative length, then null will be returned:

str('Hello, world!')->charAt(100); // null
str('Привет, мир!')->charAt(-13); // null
str('「こんにちは世界」')->charAt(-2000); // null

Why it do so

Str provides a better way to work with strings in Laravel, because it always work with multibyte strings.
For now there is no way to simply get character by index if you work not only with latin characters.

$string = 'Привет';
$string[0]; // not a 'П'

// ArrayAccess was recently added for Stringable
str($string)[0] // not a 'П'

// To actually get a 'П' we need to do the following
$cyrillic_p = mb_substr($string, 0, 1); // 'П'

// I think the following is much better
Str::charAt($string, 0); // 'П'
// or
str($string)->charAt(0); // 'П'

Behavior I would like to discuss

Current implementation will return null if index is greater then string length (or if negative index less then negative string length). null indicates that string does not contain character at that index. For me this behaviour seems to be more strict then returning empty string (''). If you see more meaning in returning empty strings, please describe why.

@taylorotwell taylorotwell merged commit 7626e3b into laravel:10.x Mar 5, 2023
@rodrigopedra
Copy link
Contributor

@taylorotwell

Shouldn't this, as a method that return a string, return a new Stringable instance instead?

Someone could be iterating a list of names to grab its first letter and ensure it is uppercase, and try this:

Str::of($name)->charAt(0)->upper();

Of course, fixing this is as simple as inverting the method's call, but I wonder if we should return a new instance for consistency with all the other methods which return a string.

Before this PR, other than Stringable@value, Stringable@toString, Stringable@jsonSerialize, and Stringable@__toString, where one would expect a static type to be returned, only the Stringable@excerpt method returns a raw string, which maybe could also be reviewed to keep it consistent with all the other methods.

@localusercamp
Copy link
Contributor Author

Recently ArrayAccess was added to Stringable. I think we could return static from charAt if ArrayAccess will use charAt under the hood but return string instead:

public function offsetGet($offset)
{
    return $this->charAt($offset)->value();
}

Then it will be consistent. What do you think? @rodrigopedra

@rodrigopedra
Copy link
Contributor

Yes, I think returning a string from offsetGet is fine, as that method allows the user to use the instance as a raw string, and any user would be less surprised to find out they get a char back.

That would be a nice middle ground solution, keeping the method consistent with others, while allowing direct multibyte character access.

@localusercamp
Copy link
Contributor Author

@taylorotwell What do you think about that? Should I create new PR to allow ArrayAccess work with charAt and change return type of charAt?

@localusercamp localusercamp deleted the charAt branch March 6, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants