NYCPHP Meetup

[Michael B Allen]

On 1/11/08, Brian D. <brian at realm3.com> wrote:
>         if (
>                 $yourObj->loadObjectByRecordId($record_id)
>                 && is_array($restrictedItems = $yourObj->getUnrestrictedItems()
>                 && $yourObj->updateUnrestrictedItems(self::ObjectItemStatus)
>         ) {
>                 $this->setRecordStatusId($yourObj->getRecordStatusId());
>                 $this->setRestrictedItemList($restrictedItems);
>                 return true;
>         }
>
>    return false;
> }
>
> Since the && operator is executed in the order you want, extra if()s
> are unnecessary.

I see three problems with this:

1) Eliminating a few if()s does not make the code better and in this
case I think it actually makes it more difficult to understand at a
glance.

2) If you have a new block for each successful step, any "undo" code
will fit gracefully:

public function checkForSomething($record_id) {
   $yourObj = new SomeObject;

   if ($yourObj->loadObjectByRecordId($record_id)) {
       if (is_array($restrictedItems = $yourObj->getUnrestrictedItems())) {
           if ($yourObj->updateUnrestrictedItems(self::ObjectItemStatus)) {
               $this->setRecordStatusId($yourObj->getRecordStatusId());
               $this->setRestrictedItemList($restrictedItems);
               return true;
           }
           // undo something from getUnrestrictedItems()
       }
       // undo sometihng from loadObjectByRecordId()
   }

   return false;
}

3) If you're trying to debug using a few log statements you might have
to breakup the condition.

Mike

-- 
Michael B Allen
PHP Active Directory SPNEGO SSO
http://www.ioplex.com/