Error handling in controller (MVC structure)

Question

Lately, I'm using domain model pattern, and my controllers methods often looks like (yii2 example, verify user in loyalty system with Ajax response).

if (LoyaltyDomModel::loyaltyVerification($post_data)) {

    if (LoyaltyDomModel::setVerify(Yii::$app->user->identity, $post_data)) {

        return  ['body'=>'successfull','status' => true];
    }
    else {
        return  ['body'=>'unable update user','status' => 'error'];
    }
}
else {
    return  ['body'=>'errorcardno','status' => 'error'];
}

If logic is more complex I have a lot of if statement with model methods which return bool values, for example (I'm trying to decouple models logic in atomic methods), and it looks like not healthy. Should I encapsulate logic of return errors in models? What is best practices?

Welcome to Code Review. "Too many if statements" is one of our most common inappropriate question titles. Our convention is to state what the code accomplishes, and show the code in context — preferably your entire function, for something like this. See How to Ask. — 200_success
– 200_success, Commented Mar 1, 2017 at 15:20
IMO, I'd push those if statements into a trait class and call them individualistically during the controllers execution. — David J Eddy
– David J Eddy, Commented Mar 1, 2017 at 15:43

Eternal1 · Accepted Answer · 2017-03-01 16:07:35Z

4

I advice you forego if checks completely and go for try-catch blocks, and have your verifiers throw exceptions on error. So, you do something like this:

try {
     LoyaltyDomModel::loyaltyVerification($post_data);
     LoyaltyDomModel::setVerify(Yii::$app->user->identity, $post_data);

     return  ['body'=>'successfull','status' => true];
} catch (LoyaltyVerificationException $e) {
     return ['body' => $e->getMessage(), 'status' => 'error']
} catch (UserUpdateException $e) {
     return ['body' => $e->getMessage(), 'fields' => $e->getFields(), /* you can pass custom info to exceptions, right? */, 'status' => 'error'];
}

Also, it's not a good idea to return bool in one case ('status' = true) and string in another ('status' = 'error');

answered Mar 1, 2017 at 16:07

Eternal1

2864 silver badges9 bronze badges

\$\begingroup\$ A generally good suggestion, but I would be somewhat concerned about mixing expected application events and code paths (like a user verification process failure, input data validation, login verification, etc.) with those that truly represent some unexpected exceptional state in your application (like failing to get a database connection). In this case it is unclear from OP whether these else conditions truly represent application state failures or validation-type failures which would be expected in normal application execution. \$\endgroup\$

Mike Brant
– Mike Brant

2017-03-01 19:05:51 +00:00
Commented Mar 1, 2017 at 19:05

Add a comment |

Stack Exchange Network

Error handling in controller (MVC structure)

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Error handling in controller (MVC structure)

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions