Re: Re: Adding `final class Deque` to PHP

On Wed, Feb 2, 2022, at 9:14 PM, tyson andre wrote: > Hi Larry Garfield, > >> >> Returning $this would resolve that, I think.  (Making it return a new, immutable >> >> copy of the Deque would be even nicer, but I realize that's probably not an argument I'm >> >> going to win at this point on this RFC.) >> > >> > Technically, you still can have single-expression usages in >> > readable/unreadable ways >> > >> > - [$deque->shift('value'), $deque][1], or >> > - ($deque->shift('value') ?: $deque), or >> > - my_userland_helper_shift_and_return($deque, >> > 'value') >> >> ... Those are all grotesque. > > True, I wasn't familiar with that event and whether your goal was to > write as few lines as possible or as quickly as possible vs in a > readable way to people familiar with php. > That clears that up. My goal with the AoC series was to 1. Demonstrate how functional-friendly PHP today is and 2. Identify places where it is still not fully FP-friendly so we can fix those. Full writeup of my results here: https://peakd.com/hive-168588/@crell/aoc2021-review (Assistance with any of those points raised is very welcome and requested.) >> > My personal preference is against making this fluent. >> > I'd rather expose an efficient datastructure that's consistent with the >> > rest of PHP's functionality to the extent possible, >> > which userland can use to write their own fluent/non-fluent classes. >> > There's drawbacks to returning $this, including: >> > >> > 1. Inconsistency with existing APIs making remembering what does what >> > harder. Barely anything in php that I remember returns $this. >> > >> >    >> >https://www.php.net/manual/en/arrayobject.append.php returns void. >> > >> >    >> >https://www.php.net/manual/en/function.array-push returns an int. >> >> So it's already inconsistent, and frankly neither of those returns is useful.  If the >> existing convention is "inconsistently unhelpful", then let's go ahead and make it >> helpful since "it's lame but at least it's consistent" is not an argument >> here.  (Sometimes that is a valid argument, but not in this case.) > > I won't know if anyone is strongly against fluent interfaces belonging > in core, or strongly against adopting fluent interfaces in an ad-hoc > way that would make it unclear if the vote was for/against the overall > functionality or the new design choice. > > I'd rather not do this without a policy RFC such as the one that passed > for namespaces ¡­{³—?ÔŠ\,º¥* > https://wiki.php.net/rfc/namespaces_in_bundled_extensions I wouldn't call anything that returns $this "fluent". "Fluent" is specifically trying to make an API read like English, and most of the APIs I see that really lean into that heavily are multi-object and end up being godawful. This is simply "there's nothing else to return, so may as well return $this", which is extremely common. (Auto-generated Doctrine entities do that almost universally for setters.) In this case, doing so also has a specific additional benefit beyond "might be nice". As far as data collection, welcome to life in RFCs. Forming consensus on something before the vote happens is basically impossible, so you have to design the best API based on limited feedback and then hope. It sucks, but that's neither here nor there. At the moment I am leaning toward voting No if the return value remains void, because that cuts the usefulness of the object in half. (Though the performance note gives me pause; that's the only thing that does.) > If you expect this to be widely considered a better design php should > adopt, please create an RFC after the vote finishes (if it passes) > before the feature freeze outlining which methods of > Collection\Deque > would change and how. Such an RFC would almost certainly fail without support from the original author (you). >> >    Developers might have several guesses/assumptions based on their >> > experience with other methods in php/elsewhere >> > >> >    - It returns the new count (JavaScript Array.push, array_push) >> >    - It returns $this (Ruby) >> >    - It returns a lazy copy, like you'd wanted, not modifying the >> > original >> >    - It's returning void and the code in question is shorthand for >> > return null. >> >      (Python, C++ >> > >> > https://www.cplusplus.com/reference/vector/vector/push_back/ , >> > offsetSet and spl push()/shift() methods) >> >> Once again, the lack of a consistent pattern means we don't need to follow the >> pattern.  As with the first point, "it's silly but at least it's consistent" is >> a valid argument in many cases, and we've changed the language accordingly before.  (Ternary >> associativity order comes to mind.)  That's not the case here.  There is no broad-based >> ingrained training that people have that would lead them to expect a given return, so we can and >> should go for whichever one is most powerful/flexible/conducive to good code.  IMO, that's >> returning $this.  > > I continue to disagree for my previously stated reasons. > >> (Again, since a purely functional version is apparently off the table..) > > I'd be in favor of immutable datastructures in core, but I don't think > I'd have much success in an RFC. > I've only heard from a few voters that are passionately for them, I'm > not sure if that translates to widespread interest. "A language teaches you to not want what it doesn't provide." - Most features don't have widespread grassroots demand for them until they happen, and then they're super common. (Some do, but most don't.) As above, we don't know if anything is going to have enough support to pass until the vote starts. This is a known design flaw in PHP's development process, independent of any particular RFC. > Though interestingly (and unrelated to this RFC > https://wiki.php.net/rfc/deque ), they do seem possible to > implement > with better asymptotic performance than I'd have expected. > Google searching mentioned Relaxed Radix Binary Trees, an immutable > datastructure surprisingly claiming performance near arrays in most > operations. > https://hypirion.com/musings/thesis > https://github.com/hyPiRion/c-rrb Sounds like we should add them. :-) --Larry Garfield