1
\$\begingroup\$

I have some questions on how I can improve this "add action" (method) in "controller":

  1. I'm using the add action only if post request. Is it correct?

  2. This action doesn't have views ($this->autoRender = false;). Is it correct?

  3. I set a response .json file to this action but I didn't change on routes to routing .json files (the file will be return when access localhost:8765/users/add). Is it correct?

  4. I'm using Enums(handmade) to store messages that will returned to user. Is it correct?

  5. I'm using an object to store the fields of message (that will returned to user), that object will be serialized and returned like this:

    $this->response->body(json_encode($response));
    

    Is it correct?

Controller code:

public function login()
{
    $this->autoRender = false;
    $this->response->type('json');
    if ($this->request->is('post')) {
        $user = $this->Auth->identify();
        if ($user) {
            $this->Auth->setUser($user);
            $response = new ResponseMessage();
            $response->code = CodeEnum::LOGIN_GRANTED;
            $response->name = NameEnum::LOGIN_GRANTED;
            $response->type = TypeMessageEnum::SUCCESS;
            $this->response->body(json_encode($response));
        }else {
            $response = new ResponseMessage();
            $response->code = CodeEnum::LOGIN_DENIED;
            $response->name = NameEnum::LOGIN_DENIED;
            $response->message = MessageEnum::USER_PASS_INCORRECT;
            $response->type = TypeMessageEnum::ERROR;
            $this->response->body(json_encode($response));
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Please review the documentation. Were you to need that controller action to work with different response formats (i.e. consider this case, I'm not asking if you do need this now) there should be no code changes required in the controller; right now it's hardcoded to json. \$\endgroup\$ – AD7six Dec 4 '15 at 9:48
  • \$\begingroup\$ @AD7six I saw this page but I only need the view with json, but follow the documentation I can't make this work, (only hardcoded works) \$\endgroup\$ – Ricardo Dec 4 '15 at 13:41
  • \$\begingroup\$ Without the ResponseMessage class it's not really clear what you're doing - it looks like a class that duplicates what the json view class already does. To perform a code review needs a complete picture, rather than just a snapshot of code that doesn't work in issolation. \$\endgroup\$ – AD7six Dec 4 '15 at 17:10
  • \$\begingroup\$ @AD7six You saw my answer? I chance some part of code \$\endgroup\$ – Ricardo Dec 4 '15 at 17:14
1
\$\begingroup\$

If you have another point to improve, please comment or answer, I will change the accepted answer

To put in CakePHP pattern I remove both lines:

$this->autoRender = false;
$this->response->type('json');

and change how I create and return the JSON file change this:

$this->response->body(json_encode($response));

to this:

$this->set('response', $response);
$this->set('_serialize', 'response');

the result:

public function login()
{
    if ($this->request->is('post')) {
        $user = $this->Auth->identify();
        if ($user) {
            $this->Auth->setUser($user);
            $response = new ResponseMessage();
            $response->code = CodeEnum::LOGIN_GRANTED;
            $response->name = NameEnum::LOGIN_GRANTED;
            $response->type = TypeMessageEnum::SUCCESS;
            $this->set('response', $response);
            $this->set('_serialize', 'response');
        }else {
            $response = new ResponseMessage();
            $response->code = CodeEnum::LOGIN_DENIED;
            $response->name = NameEnum::LOGIN_DENIED;
            $response->message = MessageEnum::USER_PASS_INCORRECT;
            $response->type = TypeMessageEnum::ERROR;
            $this->set('response', $response);
            $this->set('_serialize', 'response');
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.