My first attempt at this question was too theoretical, so I've rewritten it with actual code. See the edit history if you care.
Supposing this logic, "suffering" from the arrow anti-pattern:
/**
* Function:effective
* ... the DateTime the org should be considered expired (or null if never).
*
* If a non-group org has a parent, inherits the parent's properties, and
* the parent isn't a root org, return the parent's expiration date.
* Otherwise, return the org's expiration date.
*
* Compare to <get>, which always returns the org's expiration date.
*/
public function effective() {
if (! $this->org->isGroup()) {
if (($parent = $this->org->getParentOrg()) instanceof OrgModel) {
if ($this->org->isParentPropertyInherited()) {
if (! $parent->isRoot()) {
return $parent->expiration()->effective();
}
}
}
}
return $this->get();
}
I have rewritten this a few other ways (including the "fail early" approach from this questionthis question), but the one that seems the most clear to me is:
public function effective()
{
$nonGroup = (! $this->org->isGroup());
$hasParent = $nonGroup && ($parent = $this->org->getParentOrg()) instanceof OrgModel;
$inherits = $hasParent && ($this->org->isParentPropertyInherited());
$nonRoot = $inherits && (! $parent->isRoot());
if ($nonRoot) {
return $parent->expiration()->effective();
} else {
return $this->get();
}
}
In my opinion, the logic tests are compact and simple, but verbose because of the method names and the language overhead of calling methods.
I am reluctant to go with my second, re-written approach because it may be obscure and not hold up under maintenance. On the other hand, I don't like the nested if. I didn't like the fail-early approach, because it repeated code.
Are there other options I'm not considering here?
