CODEWITHSUNDEEP
symfonycode-duplication
Faery
Faery

Reputation: 4650

Code duplication - Symfony2

I'm using Symfony2 and Doctrine and I have several lines duplicated in almost all of the methods of my controller.

Here they are:

$this->expense = $this->expenseRepository->findByExpenseId($id);

    if(!$this->expense){
        echo 'There is no expense with such id...';
        //redirect to error page
    } else {
        return $this->expense = $this->expense[0];
    }

I couldn't think a better way to avoid it than this:

private function findExpense($id)
{
    $this->expense = $this->expenseRepository->findByExpenseId($id);

    if(!$this->expense){
        return $this->redirect .... ;
    } else {
        return $this->expense = $this->expense[0];
    }
}

and then in each method something like this :

    $expense = $this->findExpense($id);        
    if (!$expense) {
        return $expense;
    }

but I'm not quite sure it's ok. Can you please give me some ideas how to improve this and get rid of the duplicated code?

Upvotes: 0

Views: 322

Answers (1)

ferdynator
ferdynator

Reputation: 6410

You should move that code into a service. Like that you can access your method like this: 

$expense = $this->get('expenseFinder')->findExpense($id);

The advantage over your current method is that all your code logic would be stored in one single file. So maintaining it will be easier. Also you should never use the echo method inside your Controllers. Render an appropriate template instead or throw an exception. For your case the HttpNotFoundException seems to be the right choice.

if(!$this->expense){
    throw new HttpNotFoundException();
} else {
    return $this->expense = $this->expense[0];
}

Create a expenseFinder.php in src/[Your Company]/[Your Bundle]/Util.

class expenseFinder {

    protected $em;


    public function __construct(EntityManager $em) {
        $this->em = $em;
    }

    public function findExpense($id) {
        // your logic here...
    }

}

And register your service in the app/config/config.yml 

services:
    expenseFinder:
        class: [Your Company]\[Your Bundle]\expenseFinder
        arguments: [@doctrine.orm.entity_manager]

Now you can call it as described in the beginning of my post.

Upvotes: 1

Related Questions