Title is a bit obtuse but I didn’t know how to put it better.
We have very strict typing which is all nice, but in some cases can really be a burden to work with.
I think I can find more examples but lets take the one that started me on this:
Looks perfectly fine and understandable in my book, only catch is that createNodes second argument has to be a NodeType object. Fine, no problem, I inject me the NodeTypeManager get the NodeType and use that. Works of course, but is really cumbersome. I would argue that in user land code just having the name of a node type (coming from configuration etc.) is at least as common as already having an object. I think we should support user by allowing BOTH (I know, doesn’t look so nice and all).
I know that is a controversial topic because we virtually never allow this kind of thing. It’s not a strong personal itch but in a few places I have stumbled over this again and again and had to write a buch of additional boilerplate code that is rather unecessary.
I’m strongly opposed to mixed parameter & return types as they increase cyclic complexity and make things harder to understand and test.
In this very case I think the problem is that the Node is responsible for creating new nodes, IMO a violation to the Single Responsibility Principle. But as that’s out of scope of this discussion, I’d suggest to create additional methods if needed (or another helper like Node::getNodeType()).
No helpers please that wouldn’t really improve things for userland code, then I would also be for moving the Node creation responsibility out of the node and providing different methods there maybe.
I tend to agree to Christian, having done the same thing loads of time already.
In the same area: From time to time, I need the persistence identifier for an entity – so again additionally injecting the PersistenceManager and asking it. I know this is “clean” and all that, but still from time to time it would be extremely pragmatic to be able to just call getIdentifier or s.th. like this on an entity.
@Bastian: Actually, I like the way you can work with the Node API currently, and to me it is totally fine that node creation happens in there… It just “feels right” to me. So I’d really like to discuss this further
I agree that the Node API as we have it right now is rather “Active Model” like that acts as a facade to the underlying repositories / services. The one thing I strongly dislike is the implicit side effects and persisting that feels uncontrolled, but that’s another topic.
This facade behavior causes all kinds of (internal) complexity and coupling on the one hand, but at least provides a pragmatic API usage for the user. Having polymorphic methods that are smart about the type for a parameter would feel correct for me in this regard.
And @sebastian: I totally agree to the persistence identifier example
No, please don’t. Because: now it is inconvenient, and thus people try more to avoid that. If there is a getter, people will start using the identifier all over the place, rendering a lot of the thinking that went into Flow useless.
The problem with mixed input types is the variance it adds. Because people will use the name $nodeType for both–strings and actual NodeType instances. There goes your readability. By Making them take that extra step, it enforces some thinking to get to an easier–and thus maybe cleaner–solution.
That being said: I’ll always vote against type variance, and let me remind you how much we looked forward to type hinting when there was none in PHP. And as soon as scalar type hinting is actually usable “in the wild”, I’ll do that as well.
