0

So I've read some already existing questions here (like this and this) and also I'm reading a book, but I still can't decide. Sorry for the long post, I've posted a decent amount of code here.

The routing, controller creating and action calling are working right now. Until this time I've just echo'd a simple text in the controller's action to see if it's working.

After this I've introduced a simple View class with a render() function which returns the "renderable" HTML code:

public function render()
{
    if (!isset($this->file)) {
        return "";
    }

    if (!empty($this->data)) {
        extract($this->data);
    }

    // dirty-hack: use output buffering so we can easily check if a file can be included
    // if not, simply reject the output and throw an exception (let the caller handle it)
    // if the inclusion was successfull then return with the buffered content
    ob_start();
    if (!include($this->file)) {
        ob_end_clean();
        throw new \Exception("View file \"{$this->file}\" not found");
    }

    return ob_get_clean();
}

The Controller class also have a render() and a protected setView() functions

public function render()
{
    if (isset($this->renderView)) {
        echo $this->renderView->render();
    }
}

protected function setView($view)
{
    if (!is_object($view) || !($view instanceof View)) {
        throw new \Exception("The \"view\" parameter have to be a View instance");
    }

    $this->renderView = $view;
}

An example controller with an example action:

public function viewHome()
{
    $contentView = new Framework\View("HomeView.php");

    // fill contentView with variables, using $contentView->setData("name", value)

    $this->generateView($contentView);
}

protected function generateView($contentView)
{
    if (!is_object($contentView) || !($contentView instanceof Framework\View)) {
        throw new \Exception("Invalid \"contentView\" parameter");
    }

    $layout = new Framework\View("MainLayout.php");
    $layout->setData("content", $contentView->render());

    $this->setView($layout); 
}

So actually the controller's action is assigning data to the view (Edit: actually it's not a view, more like a template, but it's my naming convention) and the view simply uses the data. This could be "reversed" so I could create View subclasses which would get a controller reference and use the controller to get/set the data to/from the presentation. For some reason I stick with the first (current) solution but I can change my mind. :)

The corresponding MainLayout.php test file is the following

<div style="margin-left: auto; margin-right: auto; padding: 5px; width: 800px; border: 3px solid green;">
    Main header <br />
    <br />
    Content: <br />
    <?php
    if (!isset($content)) {
        die("The \"content\" variable is not available");
    }
    echo $content;
    ?>
</div>

Note that the codes I've posted serve test purposes, things can be different later on. However my real problem comes with the model layer. I've read two different approach about the MVC pattern:

The first says that the model is a "stupid" layer, a simple memory/object representation of the data. The real work (so the DAO access, queries, business logic) is in the controller.

The other one says that the controllers are the "stupid" things, they are just a glue between the model and the view, the work is done by the model. The second one seems to be easier and it fits more to my current design. What do you think, which one is the preferred approach?

And another one: let's say I've choosen the 2nd approach (described above), so I have a model class for eg. a User. This class have different (static) functions for querying things, like "get all users", "add new user", and so on. However I don't want to connect the model with the Database this strong. My first idea is that I should create (at least) 3 classes for a single model:

  • an abstract class/interface which defines the "model" itself
  • at least one subclass (the implementation) for each data-access type
  • and a factory for the model which instantiates the proper class. If we need a mock model for testing, it would return a UserMock instance instead of a UserDB instance.

Do you have any better ideas?

Edit:

Based on the accepted answer, I've decided to use the service design. Here is a simple (sample) class:

abstract class UserService
{
    private static $instance;

    public static function setInstance($instance)
    {
        if (!is_object($instance) || !($instance instanceof UserService)) {
            throw new \Exception("Invalid \"instance\" parameter");
        }

        self::$instance = $instance;
    }

    public static function get()
    {
        if (!isset(self::$instance)) {
            throw new \Exception("Instance is not set");
        }

        return self::$instance;
    }

    public abstract function findAll();

    public abstract function findById($id);

    public function add($user)
    {
        if (!is_object($user) || !($user instanceof User)) {
            throw new \Exception("Invalid \"user\" parameter");
        }

        $id = $this->addAndReturnId($user);
        $user->setId($id);
    }

    protected abstract function addAndReturnId($user);
}

This way I can register a service implementation (which uses the database or just filled with test data). However for each service I should copy-paste the set/get code. I could use a base class for this but the "instanceof" check is different for each subclass.

I know this is a bit off-topic but do you have any good idea for this? Or should I copy-paste that two functions for each service?

Community
  • 1
  • 1
csisy
  • 395
  • 1
  • 13
  • 1
    TL;DR. Is this more applicable to http://codereview.stackexchange.com/ ? – Jonnix Oct 04 '16 at 13:23
  • @JonStirling it could be there as well, however I'm not asking about the posted code, I'm asking about a not-yet-implemented code. I've just wanted to show that (I think) I've got the basic idea and some things are working, sorry if it's misleading. – csisy Oct 04 '16 at 13:29

1 Answers1

1

Note: I'm working a lot with Symfony / Doctrine so my point of view is probably pretty influenced by that. But I think they are pretty well-designed libraries, so I hope this is not a problem.


Why not use both approaches? Let controllers AND models be dumb. Model classes should only be concerned about holding the data. Basically just a class with some properties and accessor methods for these properties; nothing else. Also the controllers should not contain too much code and leave all the "interesting stuff" too other classes: Services.

Symfonys documentation describes services as follows:

Put simply, a service is any PHP object that performs some sort of "global" task. It's a purposefully-generic name used in computer science to describe an object that's created for a specific purpose (e.g. delivering emails). Each service is used throughout your application whenever you need the specific functionality it provides. You don't have to do anything special to make a service: simply write a PHP class with some code that accomplishes a specific task. Congratulations, you've just created a service!

Now you could just create services like "UserRepository", "BlogRepository", "SomeModelRepository" and these services contain all the code to work with the corresponding models like loading all users, search users, storing users to the database and so on.

A big advantage: If you also create corresponding interfaces (e.g. UserRepositoryInterface) you can just exchange these repository classes in future if you want to store your model in a different DBS or even in plain text files instead of your old database. You wouldn't need to change your models or controllers at all.

Maybe you should take a look at how Doctrine handles these things.

Also take a look at the Dependency Injection design pattern if you like the idea of services.


Edit (05.10.2016)

Your solution works, so if the following is too complicated for you right now, just stick with it. But it helped me, so I'll try to advertise it. :P

Your UserService should only care about the actual user management methods and not about getting/setting instances of itself. Your approach is using a variation of the Singleton pattern and Singletons are not necessary a thing you really want to have (see here).

If you want to work with "services" you probably should get into "Dependency Injection" (as mentioned above already). It is a bit hard to get into it in the beginning, but once you know how to use it, it improves your code quality a lot (at least it did for me). This introduction seems really good: PHP: The Right Way. Symfony also provides a DI component which you could use in your project to dont bother with the implementation details: Symfony Dependency Injection component

Community
  • 1
  • 1
Tobias Xy
  • 1,946
  • 15
  • 18
  • I actually like when someone refers (or influenced by) to a well-known and working library. So the models are "Java Bean" classes, the controllers are the "presentation feeders" - as I described in the OP. Actually I've silently started to work something similar just with a little difference, I named the class to XyDAO not Service, but the Service fits better. :) I like this design decision so I guess I accept this as a solution. As a side note these classes could be the MVC classes as well where the View would be the current Controller, the Controller would be the Service. – csisy Oct 04 '16 at 16:03
  • I've updated the OP, I'd be happy if you could check it. :) – csisy Oct 04 '16 at 19:04
  • 1
    Added something to my answer as well. Dont see it as a "must do it like this", it just is a hint how I like do stuff like this. – Tobias Xy Oct 05 '16 at 11:23
  • Thanks! I'm already using the DI (also seen a nice google tech talk [video](https://www.youtube.com/watch?v=RlfLCWKxHJ0) in yt) but I'm not sure how should I actually do it with the controllers. Since the controllers are instantiated by the Router class, I'm not sure how, when and who should assign the service to the controller. – csisy Oct 05 '16 at 11:46
  • 1
    I think either you have to define all your controllers as services as well or you have to violate the inversion of control at some point (Symfony for example injects the whole service container into the abstract controller class, so you can get services by using `$this->get('servicename')` ) – Tobias Xy Oct 06 '16 at 08:47
  • The service container idea sonds cool. It also solves the issue if a controller needs more services. Also this approach can be "automatized" by defining an array of arrays, where the first key would be the name of the controller and the value would be an array of services/service names. Thanks for your replies! :) – csisy Oct 07 '16 at 08:59
  • It's me again. :) I've checked the Symfony framework. I don't want to "copy-paste" things but their index.php file looks so cool. However I'm not sure what should I do with top-level exceptions. Symfony manually includes the autoloader file, then creates a kernel, gets a http request, asks the kernel to process it then send the response. So the kernel's handle() can catch the exceptions and behave based on the environment settings (dev/test/prod). But what if the autoloader throws an exception because it cannot find the Request/Response (or our custom AppKernel) classes? – csisy Oct 11 '16 at 10:03
  • Well, if a class is missing you will get an error as if you're using a vanilla PHP application. In the 'dev' environment Symfony will register custom exception and error handlers to show a nice formatted output of the error (see web/app_dev.php file and this: http://symfony.com/doc/current/components/debug.html ) – Tobias Xy Oct 11 '16 at 13:24
  • and what happens when an exception / error occurs in a 'prod' environment? I've seen some error document templates which is rendered. But that rendering has to be called by someone / somewhere. I've done this by wrapping the body of initialize() and handle() functions in a try-catch and call a custom handleException() function which does the job. Actually I could simply register an exception and error handler function to php and never catch exceptions. – csisy Oct 11 '16 at 14:02
  • I've found the code [here](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpKernel.php#L227). But it's still unclear what happens if an exception is thrown in this function. So this function is called from the HttpKernel::handle if an exception was thrown. Based on the exception a new response object is created which is then sent. But what if the autoloader throws an exception because it cannot find eg. the Request class? I think I'm close to the solution (I'm doing something similar) but this edge-case made me stop at that point. – csisy Oct 11 '16 at 14:54
  • 1
    Exceptions happening too early (e.g. in the autoloader) are not getting special treatment by symfony. There is no difference in using Symfony or a vanilla PHP application here, but these exceptions should not happen. They indicate the project is not set up correctly or there is a programming error, so it shouldn't be in the production environment anyway. – Tobias Xy Oct 12 '16 at 21:04