Re: [RFC] [Discussion] Deprecate functions with overloaded signatures

Hi Máté, > Le 30 mai 2023 à 01:41, Máté Kocsis <kocsismate90@gmail.com> a écrit : > > Hi Claude, > >> The replacement methods for IntlCalendar::set() (namely IntlCalendar::setDate() and >> IntlCalendar::setDateTime()) must not have a return type of void, >> but true, like the original method, for the two following reasons: >> >> 1. By changing the returned value, you are introducing an unnecessary hazard when migrating >> code. >> >> 2. Per https://www.php.net/IntlCalendar, all modification >> methods of that class (clear(), roll(), setTime(), etc.) consistently return Õ‘ >> ú(y­R,ÌæòIWNÄtrue on success and false on failure, >> even when the method is infallible (and thus would always return >> true). Don’t break consistency. > > 1. I don't see much risk with this change, since we clearly indicate at the very > beginning that the new methods return void, so programmers migrating their code > should pay attention to the return types. IDEs and static analysers can surely warn them in > case of inattention. The very risk is that the programmer must be aware that the return convention has changed. Recall that not everyone run static analysers, especially for such a trivial migration, and that not every code is statically analysable (e.g.. $foo->$bar()). Of course, the benefice of the change might be worth the risk; but that leads to the second point: > > 2. Some of the methods you mentioned have a "true" return type for a few weeks now. > The biggest motivation for introducing these was to at least have some chance to > make them void in the future. Doing so is a risky move indeed, so should be carried out very > carefully, if it's possible at all... That's why I consider it beneficial to spare the > effort and start with a clean slate by adding the new methods in question with their correct > return type. Concretely, if you change the return value (from true to void/null), you will break the following pattern: <?php if (!@$foo->setBar(...)) { // error handling (might be dead code, it doesn’t matter) } ?> Any change that introduce a BC break or a migration burden must be justified. (There has been enough rant around justified BC breaks, so let’s not introduce unjustified ones.) What is the purpose of changing the return convention of IntlCalendar methods, and is it worth? —Claude