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.

Fabian Schmengler · Accepted Answer · 2013-01-22T22:18:10.260

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.

thanks this helps a lot to remove duplication in the catch clause. Is there some way to further remove the try/catch duplication? — EricC, Jan 22 '13 at 21:43
Just the `try` and `catch`? No, and it would not make sense to do so. But together with the duplicated `foreach` head, you could refactor the whole thing. Wait for my edit... — Fabian Schmengler, Jan 22 '13 at 21:58

score 0 · Answer 2 · answered Jan 22 '13 at 06:35

0

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
  }

}

answered Jan 22 '13 at 06:35

rink.attendant.6

44,500
61
101
156

Thanks for the feedback. However, I do need the try/catch nested inside the foreach loop so that when a particular iteration throws an exception, the loop continues to the next iteration instead of terminating. – EricC Jan 22 '13 at 14:22

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

2 Answers2

Linked