has() vs tryFrom()
iluuu1994 opened this issue · 7 comments
Suggested by Nikita, has() itself is rarely useful. It will always be used in combination with from() so we might as well offer tryFrom() to make these cases shorter.
Additional plus: Static analyzers will understand that tryFrom()
doesn't throw an exception while has() ? from() : null
would require additional logic to understand.
As discussed in slack, my primary concern with from()/tryFrom() is that we'd be stealth-introducing a naming convention of "tryX() means it may return null". I am highly wary of that without further consideration because the implications could be large.
(Not necessarily bad, but large.)
There's also the option of getOrDefault(string $name, $default = null)
, but both would definitely be different naming conventions. I don't have a strong preference.
Additional plus: Static analyzers will understand that
tryFrom()
doesn't throw an exception whilehas() ? from() : null
would require additional logic to understand.
I agree it's definitely easier.
They'd have to get it from an external repo such as https://github.com/JetBrains/phpstorm-stubs/blob/master/Reflection/ReflectionClass.php (or extract it) because getDocComment() is almost always null for internal classes, methods, etc.
- in comparison, https://www.php.net/manual/en/reflectionclass.getproperty.php does not reference ReflectionException anywhere
Phan and Psalm check for user-defined methods but not for internal methods (at least not that I remember for psalm)
I'd feel more comfortable with from(): self
/ tryFrom(): ?self
if voters were clear of the implications. Unless someone objects I'm going to start a thread on Internals tomorrow to highlight it and see if anyone has an issue with setting this precedent. If there's no serious pushback, then we can go with that.
The downside of fromOrDefault
is that with missing generics the return type would have to be mixed
. My preference is tryFrom
. I'm also cool with fromOrNull
.
RFC updated to use from
/tryFrom
.
Leaving open until @iluuu1994 updates the implementation. Ilija, please close when you've done so.