Action method code reuse in Silex Controllers

Question

Today while creating a simple controller for a Silex application, I noticed a number of action methods had almost identical code:

public function someAction()
{
    $id = $this->request->attributes->get('id')
    if($id)
    {
        $data = getMyDataFromDatabaseWithThisID($id);

        //do something with data

        $this->app->json($data, 200);
    }
    else
    {
        //do error stuff
    }
}

public function someOtherAction()
{
    $id = $this->request->attributes->get('id')
    if($id)
    {
        $data = getMyDataFromDatabaseWithThisID($id);

        //do something else with data

        $this->app->json($data, 200);
    }
    else
    {
        //do error stuff
    }
}

The bones of each method is pretty much exactly the same. I figured if I have 3 or 4 methods following the same pattern, I should probably abstract out some of that code.

My first instinct was to have a big executeAction($action) method, but I do not know of a way of passing a variable to the called controller action from a Silex route. Second, I figured replacing lots of smallish methods with one massive one was asking for trouble.

My next thought was to create an executeAction(Closure $action) method that was called by the existing action methods passing a closure with the task needed.

So an action method such as updateAction() could pass a closure to executeAction() telling it to update the desired resource:

public function updateAction
{
    $this->executeAction(function($resource, $request) {
        $resource->doSomethingToUpdateItFromInfoInRequest($request->get('data'));
    });
}

public function executeAction(Closure $action)
{
    $id = $this->request->attributes->get('id')
    if($id)
    {
        $data = getMyDataFromDatabaseWithThisID($id);

        //execute closure action
        $action($data, $this->request);

        $this->app->json($data, 200);
    }
    else
    {
        //do error stuff
    }
}

On the face of it this looks quite clean. It is also flexible in that it is up to each individual method whether or not executeAction is used or not. The issue I have with this idea is that I need to make sure anything done in the closure is properly reflected outside the closure (i.e. are variables/objects being updated correctly etc). It also doesn't feel like the right way to do it. Though this could be due to closures in PHP being fairly new to me (before playing with Silex I never used them in my PHP code).

The third option is to reduce some of the common parts to methods of their own - such as

public function getResourceById($id)
{
    //do stuff
    return $resource;
}

public function errorStuff($code, $message)
{
    //do stuff
}

//then in action method
public function updateAction()
{
    $data = $this->getResourceById($this->request->get('id');
    if($data)
    {
        //do stuff
    }
    else
    {
        $this->errorStuff(1001, 'uh oh!');
    }
}

The method looks smaller, but the same problem exists - I will have several methods that look virtually identical and will need to be updated if something changes (method names etc).

So, given I am using Silex and a Silex Controller class which of these approaches, if any, is the better choice for avoiding code repetition? Can anyone suggest a different solution entirely?

Answer 1

Your code above doesnt really look like a silex codebase to me, more like symfony controllers.
So i wont comment on this particulary.

I recommend however you get familiar with closures, as you already reconned, since they are heavily used in Silex.

For your purposes you might want to have a look at the Silex implementation of Param Converters.
They can take the part of your getMyDataFromDatabaseWithThisID functions. Further they could also handle errors.

You can register a findOr404 method in your app:

$app['findOr404'] = $app->protect(function($id, $message = null) use ($app) {
  //get the data or abort with 404
}

and use it as ParamConverter in your Silex Controllers:

$app->get("/get/{id}", function (Data $data) {
    // ...
})->convert("id", $app['findOr404']($id);

This example is inspired by this Blog Post where you can find some more tricks on this.

Also have a look at how to wrap your controller in classes, if you get lost in closures. This might feel a bit more familiar.