php 5.3 avoid try/catch duplication nested within foreach loop (code sandwich)

Question

I have a class with methodA, which has existing structure as following

function methodA() {
  $providers = $this->getFirstSetOfProviders();
  foreach ($providers as $provider) {
    try {
      $this->method1($provider);
    } catch ( Exception $e ) {
      // exception handling
    }
  }

  $providers = $this->getSecondSetOfProviders();
  foreach ($providers as $provider) {
    try {
      $this->method2($provider);
    } catch ( Exception $e ) {
      // exception handling
    }
  }
}

The content for the catch clauses are identical. Is there some way to organize the code to avoid repeating the structure of try/catch nested in a foreach loop? Conceptually, I am trying to do

function methodA() {
  foreach ($providers as $provider) {
    $method1 = function($provider) {
      $this->method1($provider);
    }
    $this->withTryCatch($method1);
  }
  ...
}

function withTryCatch($method) {
  try {
    $method;  // invoke this method somehow
  } catch (Exception $e) {
    // exception handling
  }
}

This looks similar to the Code sandwich, but I am not sure how to proceed in php.

UPDATE: The try/catch is nested inside the foreach loop so that when an exception is thrown, it's handled and the execution continues to the next iteration in the loop instead of terminating the loop.

Answer 1

The nice thing about Exceptions is that they are objects that can be passed around like any others. So you can remove the duplicate code (except the basic boilerplate) without changing much:

foreach ($providers as $provider) {
    try {
      $this->method1($provider);
    } catch ( Exception $e ) {
      $this->handleException($e);
    }
}

Note: If you also need some context within the exception handling (i.e. $provider), just give handleException() more parameters.

Part 2: Refactoring the whole method

You wanted to know how to further remove duplication. I don't know if this makes sense in your actual code, it could as well be over-engineering. You will have to decide that by yourself. The following is an implementation of the Template Method pattern. Forgive the crude naming but I tried to follow your example and I have no idea what you are doing.

abstract class ClassThatDoesThingsWithProviders
{
    public function methodA($providers)
    {
        foreach($provicers as $provider) {
            try {
                $this->methodThatActuallyDoesSomethingWithProvider($provider);
            } catch(Exception $e) {
                $this->handleException($e);
            }
        }
    }
    protected function handleException(Exception $e)
    {
        // handle exception
    }
    abstract protected function methodThatActuallyDoesSomethingWithProvider($provider);
}
class ClassThatDoesThing1WithProviders extends ClassThatDoesThingsWithProviders
{
    protected function methodThatActuallyDoesSomethingWithProvider($provider)
    {
        // this is your method1()
    }
}
class ClassThatDoesThing2WithProviders extends ClassThatDoesThingsWithProviders
{
    protected function methodThatActuallyDoesSomethingWithProvider($provider)
    {
        // this is your method2()
    }
}

class YourOriginalClass
{
    protected $thingsdoer1;
    protected $thingsdoer2;

    public function __construct()
    {
        $this->thingsdoer1 = new ClassThatDoesThing1WithProviders;
        $this->thingsdoer2 = new ClassThatDoesThing2WithProviders;
    }
    public function methodA()
    {
        $this->thingsdoer1->methodA($this->getFirstSetOfProviders());
        $this->thingsdoer2->methodA($this->getSecondSetOfProviders());
    }
}

You could easily make an array out of thingsdoer1 and thingsdoer2 and maybe abstract getFirstSetOfProviders and getSecondSetOfProviders alongside. Also I don't know what the actual method1 and method2 implementations depend on, maybe you cannot extract them like that without breaking cohesion.

But as I don't know your real code and what you are doing I cannot recommend a concrete strategy, regard my example above as a starting point.

Answer 2

function methodA() {
  try {
      $providers = $this->getFirstSetOfProviders();
      foreach ($providers as $provider) {
          $this->method1($provider);
      }

      $providers = $this->getSecondSetOfProviders();
      foreach ($providers as $provider) {
          $this->method2($provider);
      }
  } catch ( Exception $e ) {
    // exception handling
  }

}