Propel2
Propel2 copied to clipboard
Introduce has() for DTO like classes
https://github.com/propelorm/Propel2/pull/1624 and alike reveal a lot of the underlying nullable issues in the current code base
Instead of blindly reusing the return result, e.g. $this->getFoo()->doSth() on currently nullable ones, we should introduce a new has...() wrapper on top, and throw meaningful exception for getter if null.
For the cases currently used as finder, we need to rewrite using
$result = $x->hasFoo() ? $x->getFoo() : null
or alike. This way we can also in the future add typehints and have a cleaner API moving towards PHP8 compatibility.
is this issue still valid, with the ?-> operator in PHP8? If $x->getFoo() returns a nullable result, that seems to be a better way to go forward than throwing on empty values/relations.
If you use PHP8+, many libs are PHP7/8 for quite some time still, though. So yeah, I would consider this a code smell still in projects and it would be nice if there was a way to avoid this.
Currently https://github.com/propelorm/Propel2/pull/1849 dos the opposite, it adds new not-null return methods where needed. This keeps the current API and therefore less BC breaks for now
We should use has() none the less where we could safely change the status quo of the methods that exist. Someone would need to investigate.
Otherwise a future major (BC break) could maybe fix this up.