multiple if statements "forced" by database query result & external api response

Question

Following scenario: this is the code that resides in Ajax Controller class. It is responsible for handling input in one of the form fields.

The logic is as follows:

If user-inputted data validate as either Tax Number or Statistical Number, it's supposed to: check for presence of such data in database, and, depending on the result, either make a call to external API (in case DB query is empty) or present the results (in case of input being found in database).

If user-inputted data doesn't validate as either Tax or Statistical Number, it is supposed to perform DB query with input treated as the customer name.

Below code works as expected. However, it is long and I was forced to repeat some parts. I'm looking to cut down on lines while preserving exact same logic, but I'm highly unsure as to how to make it even more compressed.

class AjaxController {

  public function customerslistAction() {
    $filterChain = new Zend_Filter();
    $filterChain->addFilter(new Zend_Filter_Digits())
                ->addFilter(new Zend_Filter_Null());

    $input = $filterChain->filter($this->getRequest()->getParam('name'));
    $collection = new Def_Model_Collection_Customer();

    /* check if $input is TaxNumber */
    if (SugApi::isTaxNumber($input)) {
      $data['tax_number'] = $input;
      $allFound = $collection->findByTaxOrStatistical($data);

      // check and return customer with this TAX_NUMBER if it is in database
      if (count(($allFound)) > 0) {
        echo Zend_Json::encode($allFound);
        return;
      }
      // call external api if it isn't in database [catch NotFound Exception]
      else {
        try {
          $sugReport = SugApi::getByTaxNumber($input);
        } catch (\SugApi\Exception\NotFoundException $e) {
          $out['results'][] = array('value' => 'Unknown tax number.');
          echo Zend_Json::encode($out);
          return;
        }
      }
    }

    /* check if $input is StatisticalNumber */
    else if (SugApi::isStatNumber($input)) {
      $data['stat_number'] = $input;
      $allFound = $collection->findByTaxOrStatistical($data);

      // check and return customer with this STATISTICAL_NUMBER if it is in database
      if (count($allFound) > 0) {
        echo Zend_Json::encode($allFound);
        return;
      }
      // call external api if it isn't in database [catch NotFound Exception]
      else {
        try {
          $sugReport = SugApi::getByStatNumber($input);
        } catch (\SugApi\Exception\NotFoundException $e) {
          $out['results'][] = array('value' => 'Unknown statistical number.');
          echo Zend_Json::encode($out);
          return;
        }
      }
    }
    // if $input is neither TAX_NUMBER nor STATISTICAL_NUMBER, 
    // query database for customer with $input as customer_name
    else {
      $collection = new Def_Model_Collection_Customer();
      $data = $collection->fetchAll(array('name' => $this->getRequest()->getParam('name')));

      $out = array();
      foreach ($data as $val) {
        $out['results'][] = array(
          'id' => $val['customer_id'],
          'value' => $val['customer_name'],
          'info' => 'Tax Number: '.$val['customer_tax_number']
        );
      }
      $json = Zend_Json::encode($out);
      echo $json;
      return;
    }

    // parse successful external api response into Customer object and save it
    $customer = new Def_Model_Customer();
    $customer->setOptionsFromSugReport($sugReport);
    $customer->save();

    $out['results'][] = array(
      'id'    => $customer->customer_id,
      'value' => $customer->customer_name,
      'info'  => 'Tax Number: '.$customer->customer_tax_number
    );
    echo Zend_Json::encode($out);
  }
}

Answer 1

If you don't want to repeat yourself (DRY) then consider using multiple methods, and set properties ($this->collection) and/or pass parameters, as necessary. If you are finding yourself nesting if statements inside a method, it is a good indication that you should be using multiple methods.

For example, for your method, it looks like you could cut the database check into their own method.

If you have a return, then there is no need for an else statement :-)

if($a){
  return;
}
// no need for else

There is no point in setting a variable $json and echo it, just echo it as you did most of the time, but not all.

It looks as though you could break the rest response into it's own method for example with the if count > 0 return true or false to determine the action.

private function method($results){
    if (count(($results)) > 0) {
        echo Zend_Json::encode($result);
        return true;
    }
    return false;
}

Check out this video "Your code sucks, let's fix it - By Rafael Dohms" to help with understanding the above. https://www.youtube.com/watch?v=GtB5DAfOWMQ

I hope this is helpful!