1

I'm struggling on designing a solid architecture for my project.
In particular, I don't know how to handle the communication between the models and the controllers.

My goals are:

  • Following the Separation of Concerns principle and have the models as independent as possible.
  • Have internationalization support for the result messages
  • Be able to interpolate result messages with arguments (just like for the logs described in PSR-3)
  • Be able to couple result messages with a severity level and a HTTP status code
  • Be able to use the same controller for HTML and JSON responses

I've read a LOT of code bases and none I've seen does properly all of the above.
Either because don't do error handling at all or because they just do all the stuff in the model.

So I've come to this approach, but I'm not sure if I'm doing it right or if I've done something terrible.

First of I'll list here the main aspects of my code:

  • The Model is decoupled from all other classes
  • The only thing that is "coupled" in the model is it's the structure of $results
  • The controller glues together the various layers of the application; it calls the controller and just sets things up in the right place, ready for building a response, may it be a raw JSON (for an API response) or may it be a HTML response built with a template engine.
  • The result messages are internationalized and can be further customized with some arguments. (e.g. "Yo, welcome back {user_name}" or "There was an error while processing the array named {name} at this key index: {key}")

Here's an example of a controller, UserController:

    class UserController {
    
        public function __construct() {
            private User $user,
            private ResponseHandler $response_handler
        }
    
        public function changeAvatar() {
            $this->user->changeAvatar( /* arguments */ );
            $this->handleResults();
        }
    
        private function handleResults() {
            // NOTE: this function *could also be put in the front controller / main caller
            // so that we don't have to repeat this in every controller.
            // *should/is, I'm placing it here so you can see how it works
    
            $map = array(
                User::ERROR_INVALID_AVATAR_EXTENSION => array(
                    400,
                    TEXT_INVALID_AVATAR_EXTENSION,
                    LogLevel::ERROR
                ),
                User::ERROR_OVERSIZED_AVATAR => array(
                    // 413 = payload too large, idk if it fits here
                    // anyway it's just an example
                    413,
                    TEXT_OVERSIZED_AVATAR,
                    LogLevel::ERROR
                ),
                User::ERROR_CANNOT_CREATE_AVATAR => array(
                    500,
                    TEXT_CANNOT_CREATE_AVATAR
                    LogLevel::ERROR
                )
                User::STATUS_AVATAR_CREATED => array(
                    // 201 = created
                    201,
                    TEXT_AVATAR_CREATED,
                    LogLevel::INFO
                ),
                User::STATUS_AVATAR_UPDATED => array(
                    200,
                    TEXT_AVATAR_UPDATED,
                    LogLevel::INFO
                )
            );
    
            foreach($this->user->results as $result) {
                if(in_array($result[0], $map)) {
                    $this->response_handler->setStatusCode($map[$result[0]][0]);
                    // addMessage( level, message, interpolation args )
                    $this->response_handler->addMessage($map[$result[0]][2], $map[$result[0]][1], $result[1]);
                } else {
                    $this->response_handler->setStatusCode(500);
                    $this->response_hanlder->addMessage(TEXT_UNDEFINED_RESULT);
                }
            }
    
            // Then in the caller we one of these:
            // $response_hanlder->buildJSONResponse();
            // $response_hanlder->buildHTMLResponse();
        }
    }

And here is the User model:

    class User {
        public $results = array();
        public const ERROR_INVALID_AVATAR_EXTENSION = 0;
        public const ERROR_OVERSIZED_AVATAR = 1;
        public const ERROR_CANNOT_CREATE_AVATAR = 2;
        public const STATUS_AVATAR_CREATED = 3;
        public const STATUS_AVATAR_UPDATED = 4;
    
        public function changeAvatar( /* arguments */ ) {
            if( /* invalid extension  */) {
                $this->results[] = array(
                    self::ERROR_INVALID_EXTENSION,
                    array( /* details for the interpolation message */)
                );
                return false;
            }
            if( /* oversized avatar  */) {
                $this->results[] = array(
                    self::ERROR_OVERSIZED_AVATAR,
                    array( /* details for the interpolation message */)
                );
                return false;
            }
            try {
                if( /* avatar exists */ ) {
                    /* update avatar */
                    $this->results[] = array(
                        self::STATUS_AVATAR_UPDATED,
                        array( /* interpolation args */)
                    );
                } else {
                    /* create avatar */
                    $this->results[] = array(
                        self::STATUS_AVATAR_CREATED,
                        array( /* interpolation args */)
                    );
                }
                return true;
            } catch( /* idk exception */ ) {
                if( /* cannot create avatar */) {
                    $this->results[] = array(
                        self::ERROR_CANNOT_CREATE_AVATAR,
                        array( /* details for the interpolation message */)
                    );
                }
                return false;
            }
        }
    }

So what do I feel bad about this approach?
The $results array. It's structure is "tied" to the models, and this could be considered a form of coupling although it relies on the language's primitives.
I could write a wrapper class for the results, like class Result{ public int $error_code; public array $interpolating_args; }, but that would mean tying the models to this new class and I don't like it.

What about trying different approaches?
For interpolating the strings we need the arguments, which are in the model, and all the ways to get out those arguments are kinda ugly.
The alternative would be to interpolate the result messages directly within the model, but this would couple the models with the string interpolation code.

I don't see any other way.
I'm a bit skeptical about adding more abstraction layers to the models; I could extract the error codes from the model class to build an ad-hoc error class for every model, though with this I would triplicate the models files and the core logic behind the whole thing would be the same.
See the code below:

class UserErrorType {
    public const ERROR_INVALID_AVATAR_EXTENSION = 0;
    public const ERROR_OVERSIZED_AVATAR = 1;
    public const ERROR_CANNOT_CREATE_AVATAR = 2;
    public const STATUS_AVATAR_CREATED = 3;
    public const STATUS_AVATAR_UPDATED = 4;
}
class UserError {
        public UserErrorType $type;
        public array $interpolation_args;
    public function __construct(UserErrorType $type, array $interpolation_args){
        $this->type = $type;
        $this->interpolation_arg = $interpolation_arg;
    }
}
2
  • comments are welcome!
    – yes sure
    Dec 24 '21 at 17:06
  • My bad sorry @candied_orange . As a non native English speaker sometimes I get confused.
    – yes sure
    Dec 29 '21 at 11:36
0

Quick Answer.

Did a similar thing, years ago.

In case of the $results, go for the O.O. wrapper approach. And, yes it will be more code, but more controlled.

Specially, when you have too many parameters, and those parameters altought displayed as text, are originally of different type, and the code of conversion can be handle in that new class, called by the UserController class, instead of the UserController class itself.

And also getting the localized text.

In case of the HTML & JSON responses, you would need also to have unleast three classes, a base class, plus each subclass.

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.