Re: [Discussion] grapheme cluster for str_split function

Hi, Larry Hi, Niels 2024年3月6日(水) 6:47 Niels Dossche <dossche.niels@gmail.com>: > > Hi Larry > Hi Yuya > > So first of all, I meant the error handling in cases like these: > https://github.com/php/php-src/pull/13580/files#diff-b8fe038d9d7539694593978bea5605f38dde4bcb6a016865130590e45e23202eR852-R860 > The implementation still returns NULL here, so the signature is still incorrect. Either it > should return false to match the other functions, or throw something and not return a value. > > On 05/03/2024 18:40, Larry Garfield wrote: > > On Tue, Mar 5, 2024, at 7:25 AM, youkidearitai wrote: > >> 2024年3月5日(火) 5:52 Niels Dossche <dossche.niels@gmail.com>: > >>> > >>> Hi Yuya > >>> > >>> This sounds useful. > >>> > >>> I do have a question about the function signature: > >>> function grapheme_str_split(string $string, int $length = 1): array {} > >>> > >>> This always returns an array. > >>> However, looking at your PR it seems you return NULL on failure, but the return > >>> type in the signature isn't nullable. > >>> Also, from a quick look, it seems other functions return false instead of null on > >>> failure. So perhaps the return type should be array|false. > >>> > >>> What do you think? :) > >>> > >>> Kind regards > >>> Niels > >>> > >>> On 03/03/2024 00:21, youkidearitai wrote: > >>>> Hi, Internals > >>>> > >>>> I noticed PHP does not have grapheme cluster for str_split function., > >>>> Until now, you had to use the PCRE function's \X. > >>>> > >>>> Therefore, I try create grapheme_str_split > >>>> function. > >>>> https://github.com/php/php-src/pull/13580 > >>>> It is possible to convert array per emoji and variation selectors using ICU. > >>>> > >>>> If it's fine, I'll create an RFC. > >>>> > >>>> Regards > >>>> Yuya > >>>> > >> > >> Hi, Niels > >> > >> Thank you for your comment. > >> Indeed, returns false is make sense. > >> > >> Therefore, I changed to returns false when invalid UTF-8 strings. > >> > >> Regards > >> Yuya > > > > Many legacy functions return false on error, but that is widely regarded as bad design. > > Please do not continue bad design. > > I agree that returning false on error isn't ideal for exceptional cases, that's what > exceptions are for. > Looking at the other grapheme functions makes me wonder though how consistent this would be, > especially w.r.t. intl_get_error_*() and intl_error_name(). > > > > > Right now, the best "standard" error handling mechanism available is exceptions. > > false (or null) can very easily lead to incorrectly using that value as though it were valid, when > > it's not, which will sometimes cause a fatal error and sometimes cause a security leak. > > > > If the input value cannot be logically processed, that's an exception. (Or Error, > > perhaps.) > > > > --Larry Garfield > > Kind regards > Niels Thank you so much for advice. Indeed, This current grapheme* functions seems inconsistent. Therefore, it's one thing when returns null, throws any exception. Shall we do so just for the grapheme_str_split function? Regards Yuya -- --------------------------- Yuya Hamada (tekimen) - https://tekitoh-memdhoi.info - https://github.com/youkidearitai -----------------------------