HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Admin method in CakePHP to add a venue to the database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
venuetheadminmethoddatabasecakephpadd

Problem

I have a VenuesController and an EventsController. My venues controller has an admin_add method that, well, allows an administrator to add a venue to the database. It looks like this:

public function admin_add() {
    if ($this->request->is('post')) {
        if ($this->Venue->save($this->request->data)) {
            $message = 'Venue has been created.';
            $this->response->statusCode(201);
            if ($this->request->params['ext'] == 'json') {
                $this->set('id', $this->Venue->id);
                $this->set('_serialize', array('id'));
            }
            else {
                $this->Session->setFlash($message, 'flash', array('class' => 'success'));
                $this->redirect(array('action' => 'index'));
            }
        }
        else {
            if ($this->request->params['ext'] == 'json') {
                $this->set('validationErrors', $this->Venue->validationErrors);
                $this->set('_serialize', array('validationErrors'));
                $this->response->statusCode(400);
            }
        }
    }
}


As you can see, there are a lot of if () statements which seem like code smells to me. But they’re there because in my CMS’s events section, you can add a venue inline via a modal, which sents the request to this function via AJAX.

So my question is: can I improve the above VenuesController::admin_add() method, make it more intelligent without the need for the if () statements, and just generally make it more CakePHP-ier?

Solution

Why do you have the following lines repeating:

if ($this->Venue->save($this->request->data)) {
        if ($this->Venue->save($this->request->data)) {


I think you may try to merge them to one and the associated logic too.

Code Snippets

if ($this->Venue->save($this->request->data)) {
        if ($this->Venue->save($this->request->data)) {

Context

StackExchange Code Review Q#30682, answer score: 2

Revisions (0)

No revisions yet.