On Sat, Dec 11, 2021 at 7:15 AM Dan Ackroyd <dan...@basereality.com> wrote:
> > There are quite a few downstream costs for making a new type of > methods in classes. All projects that analyze code (rector, > codesniffer, PHPStorm, PhpStan/Psalm, PHPUnit's code coverage > annotations etc) would have to add a non-trivial amount of code to not > bork when reading the new syntax. Requiring more code to be added and > maintained in PHP's builtin Reflection extension is also a cost. > That's quite a bit of work for a feature that has relatively rare > use-cases. > While I'm not suggesting that this isn't an important consideration for voters with this RFC, (I think it should be weighed for sure), I *think* that all of the things you mentioned will need similar updates to work correctly with this RFC even if it was done with plain old magic methods instead. Again, I'm not saying it's not important, but isn't this true of many RFCs that add new syntax, such as enums or even things like array unpacking? As for Reflection, I hadn't gotten to that yet, but I actually pushed the commit for it with full test coverage of the changes last night. It wasn't actually that big of a change, and the latest commit on the PR can be checked out and played if you want to test it. With support for the enum casing as well. Operators get an additional function flag ZEND_ACC_OPERATOR that made it fairly simple to implement the Reflection changes with minimal additional code. The new methods I mentioned are actually smaller that the implementations for the existing ones. As far as maintainability goes, removing this aspect doesn't make this RFC more maintainable in core in my opinion. It becomes harder to maintain it I think, as it requires more special casing in other places that is more obtuse and obscure. > I don't get this. The magic methods in previous drafts of the RFC > don't have a problem with & as the methods are named with 'two > underscores' + name e.g. __bitwiseAnd. That does't appear to cause a > problem with an ampersand? > This was referring to continuing to use symbol names, but without a new keyword. The name of the function (e.g. __add) always refers to the symbol used > where it is used, not what it is doing. > > If the code is `$a + $b` then that is an addition operator, when the > code is read. If I was reading the code, and I saw that either $a or > $b were objects, I would know to go looking for an __add magic method. > True, and I don't think it would be *ambiguous* this way. However, method names for other methods tend to describe what the function does. > > " '// This function unions, it does not add'" > > Then that is probably an example of an inappropriate use of operator > overloads, and so shouldn't be used as a justification for a syntax > choice. > Larry's example with dot-product and cross-product is a better example, but one that is less accessible to those who don't know vector math/linear algebra. I used this example because this is exactly how PHP treats + for arrays, so I would think most Collections would implement + as the union operator in order to remain consistent with internals. > I think this is just wrong, and makes the RFC unacceptable to me. > > Although most of the code I write is code that just performs > operations as I see fit, some of the time the operations need to be > driven by user data. Even something simple like a > calculator-as-a-service would need to call the operations dynamically > from user provided data. > I'm confused why this wouldn't still be possible? First, you can still get the closure for the operator implementation from Reflection if you really, really need it. But second, with user data couldn't you use a setter to change the object state prior to the op, use a method specifically for calling as a method, or just combine them with the operator? `$obj->setValue($userData);` `$result = $obj + $userData;` > > I also have an aesthetic preference when writing tests to be explicit > as possible, rather than concise as possible e.g. > > $foo->__add(5, OperandPosition::LeftSide); > $foo->__add(5, OperandPosition::RightSide); > > instead of: > > $foo + 5; > 5 + $foo > > As I find that easier to reason about. > > This I can understand. Again, I don't think you're wrong here, I think this is a matter of opinion and taste. I can understand your position, but I think the long-term maintainability and support the keyword offers is worth the short term pain. Larry: > I largely agree here. I don't know if it's because of the `operator` choice or not, but being able to call an operator dynamically is important in many use cases. It doesn't have to be a pristine syntax, but some way to do that dynamically (without having a big match statement everywhere you need to) would be very welcome. Overall, the point in making them non-callable was to force PHP developers to stop thinking about these as methods. They are modifications to engine behavior that are given directly to PHP devs. Using them as methods would *usually* indicate incorrect usage. If that's what you need, then probably you need a method, not an operator overload. If you need *both*, then what I would suggest is implementing the logic as a normal method, and then calling that method inside of the operator overload. > Another question: Can an interface or abstract class require an operator to be implemented? That's not currently discussed at all. (I would expect the answer to be Yes.) Yes. Both abstract classes and interfaces can require implementations of the operator keyword as they would with a method. Jordan
