Expand String functions#77
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Al2Klimov
left a comment
Al2Klimov
left a comment
There was a problem hiding this comment.
Please hand over to the next reviewer ASAP!
|
Hi, I don't think, I can give any meaningful reviews on this, thus, it would probably be better to request a review from @nilmerg instead, if Eric is not available. |
lippserd
left a comment
lippserd
left a comment
There was a problem hiding this comment.
One more thing to consider: This caseSensitive parameter construct is something that I would do differently nowadays. Passing false without using a named argument requires the reader/caller to read the docs. It unnecessarily bloats the contract. And it requires two different implementation blocks in each function. Consider introducing …Insensitive() variants instead.
| { | ||
| $subject ??= ''; | ||
| if (! $caseSensitive) { | ||
| return strncasecmp($subject, $end, strlen($end)) === 0; |
There was a problem hiding this comment.
This is a prefix check but it should be a suffix check. Tests don’t exercise the contract properly. You always pass true, which is the default.
| * Check if the given string contains any of the specified substrings | ||
| * | ||
| * @param ?string $haystack | ||
| * @param string[] $needles |
There was a problem hiding this comment.
Use iterable to support any string iterator/generator.
| * Check if the given string contains all the specified substrings | ||
| * | ||
| * @param ?string $haystack | ||
| * @param string[] $needles |
There was a problem hiding this comment.
Same iterable consideration as above.
| * Null is considered empty and strings are trimmed before checking. | ||
| * | ||
| * @param ?string $subject | ||
| * @param string $characters |
There was a problem hiding this comment.
isEmpty should not expose implementation details. Its contract is that it considers null or whitespace only strings as empty. That’s it. Calling trim must be replaced anyway if we want this function to also work with UTF-8 inputs.
| */ | ||
| public static function contains(?string $subject, string $needle, bool $caseSensitive = true): bool | ||
| { | ||
| $subject ??= ''; |
| */ | ||
| public static function endsWith(?string $subject, string $end, bool $caseSensitive = true): bool | ||
| { | ||
| $subject ??= ''; |
4 participants
Add some static methods to the Str class.
endsWithas the counterpart to the existingstartsWithcontainsto have a case independent version ofstr_containscontainsAnyandcontainsAllto quickly compare against an array of stringsisEmptyto reliably check for empty strings