6

I'm going to take the example of a user object. A user needs to be registered, logged in, logged out, edited (e.g., email change), etc.

So on one hand I have a user object, which includes a variety of class variables (pseudo, email, etc) along with getters and setters and maybe some functions that do not deal with the db.

On the other I have a DAO class which is the object that directly deals with the database through a variety of MySQL / PDO queries (create record, update, retrieve info, etc).

Is there any reason to not have the user object interact directly with the DAO object? In other words, when the Controller request a database query relating to an existing user instance (e.g., during the registration process), should it simply call a function in user which itself calls a function in DAO, or should there be a layer in between?

I have seen examples where the controller calls a 3rd class to interact with the DAO, and passes a user instance as an arg. Sometimes instead, this third layer is in charge with creating the user instance and dealing with the DAO. It seems to me that all the functions used to deal with the DAO could reside inside the user object. What am I missing?

JDelage
  • 13,036
  • 23
  • 78
  • 112

2 Answers2

3

If you are adhering to MVC design pattern, then there is not reason for the User instance to be in the controller. It should be part of model layer, instead of leaking over to presentation layer.

It seems that what you call "controller" is actually more of a service, which should be a part of the model layer, that handles the interaction between domain objects on the data storage related structures (mappers, repositories, DAOs).

Basically, what you are missing is correct separation of concerns.

The controller should be just passing the data to recognition or some user management service, instead of dealing with domain business logic. Said service should initialize the User object, validate the data and try to save it. Controller should no be aware of any of this.

Something like this:

class SomeController
{
     // ---- snip ----

     public function postRegister( $request )
     {
          $accounts = $this->serviceFactory->create('AccountManagement');
          $account->create( $request->getPost('username'),
                            $request->getPost('email'),
                            $request->getPost('password'),
                            $request->getPost('password2') );

          $this->view->setState( View::STATE_POST );
     }

     // ---- snip ----
}

class AccountManagement extends Service
{
     // ---- snip ----

    public function addUser( $username, $email, $password, $repeated_password )
    {

        $user = $this->domainObjectFactory->create( 'User' );

        $user->setNickname( $username );
        $user->setEmail( $email );
        $user->setPassword( $password );
        $user->matchRepeatedPassword( $repeated_password );

        if ( $user->isValid() )
        {
            $dao = $this->DAOFactory->create( 'User' );
            $dao->save( $user );
        }

        // additional code for saving the error state
        // if domain object turns out to be invalid
        // or DAO returns an error
    }

     // ---- snip ----
}

P.S. you might find this post relevant.

jeremy
  • 9,965
  • 4
  • 39
  • 59
tereško
  • 58,060
  • 25
  • 98
  • 150
  • 1
    People who downvote, should explain the reasoning. – tereško Sep 19 '12 at 11:15
  • what is the benefit of using class constants? like View::STATE_POST ? Do you use it in other places except controller and view? Why not just use $this->view->setState('post'); – Andrew Sep 17 '14 at 13:21
  • None, it is outdated post. It was when I still was misguided by all the framework crap and thought that controllers have to tell something to the views. – tereško Sep 17 '14 at 15:37
  • but do you use class constants in similar way? My question was about constants not state status :) – Andrew Sep 17 '14 at 16:06
  • Nowadays I use class constants only within the model layer for states and error codes. For example: `$user->setStatus(User::STATUS_BANNED);` or `if ($auth->hasError(Error::DUPLICATE_EMAIL)) { ...`. I basically use the class constants to abstract and explain numeric states. It also lets me change the underlying value in single place, without grepping all the code only to replace "3" with "4". – tereško Sep 17 '14 at 16:34
  • Ok, got it. Thanks. And speaking about PRG implementation in the MVC, how do you do it then? Do you store the result state of service method in the property of said Service and then check it from View or some other way..? – Andrew Sep 17 '14 at 16:42
  • I really will have to revisit this post ... Hmm ... The way I do lately is this: The controller is doing all of the alteration of model layer. If I actually write something to the model (to the persistence, to be exact), then the service which was used knows it. The associated view (I tend to have 1:1 relation between C and V) is usually interacting with same services and it checks if service did any "writes". If there were any, the view sets a location header in the response object, if not, the view does other things. – tereško Sep 17 '14 at 16:48
  • @Andrew This also affects the "writes" to services which the view does not care about. If your controller updated the tracking service, which incremented the view count on an article, then view does not care about it. It only talks to the library service which provides it with the content of the article. There was no "write" in the service with view uses, therefore, there is no need for redirect. – tereško Sep 17 '14 at 16:50
  • But if you are sending form with POST, you have to redirect to escape "F5 issue". and your first answer is exactly what i had in mind, but my question was about this part "..it checks if service did any "writes".." How does this part is implemented? Where is the status (success or fail) is 'captured' and then how View layer retrieves it? – Andrew Sep 17 '14 at 16:57
  • @Andrew looks like you missed the "If there were any, the **view sets a location header** in the response object, (..)" part. There is no F5-issue, because PRG gets implemented. – tereško Sep 18 '14 at 18:43
  • we are talking about different things. Yes, i understand the strategy and I know that view must send location header. But in order to do this, View must check if service did any writes. How is this checking implemented? Service Class has property and Service Method fills this property? Any other way? This is a question about implementation not strategy :) – Andrew Sep 18 '14 at 19:31
  • What is "service method"? Why does it matter how exactly is any of the services implemented? – tereško Sep 18 '14 at 20:15
  • 'Service Method' is the class method inside some service which view uses. And my question about implementation was, because I have to implement it somehow, but I am sure that my implementation is not the best, so I want to compare implementation ideas from other people and improve mine. For example, does the 'write status' is written inside the Service class or maybe inside Controller class? What happens when you have POST without any data to pass, but the View still needs 'write status' condition? So you have to set 'Write status' in the Controller, not Service class? How do YOU do? – Andrew Sep 19 '14 at 07:46
  • cont. I mean the write status condition is set using Service method 'set_status()', but is this method used inside Service method create_account() or inside controller method 'post_register()'? – Andrew Sep 19 '14 at 07:49
2

I think you’re describing some sort of adapter or gateway for models, where you have a class that calls an adapter (it may interact with a database, or an XML data source) and then assigns the results to instances of your model (i.e. assigns each matching user result to an instance of your User model).

In the past, I’ve taken the approach you’ve described where you have models that represent one item, and then a DAO to save and retrieve these items to a data source (be it a database or whatever), and the controller that does the logic. So an extremely basic version:

UsersDAO.php

class UsersDAO extends DAO
{
    public function save(User $user)
    {
        return is_null($user->id) ? $this->insert($user)
                                  : $this->update($user);
    }

    public function insert(User $user)
    {
        $sql = "INSERT INTO `users` (`username`, `password`)
                VALUES (:username, :password)";

        $stmt = $this->db->prepare($sql);
        $stmt->bindParam(':username', $user->username);
        $stmt->bindParam(':password', $user->password);
        $stmt->execute();
    }
}

User.php

class User
{
    public $id;
    public $username;
    public $password;
    public $email;
}

UsersController.php

class UserController extends Controller
{
    public function register()
    {
        $usersDao = new UserDAO;

        if ($_POST) {
            $user = new User;
            $user->username = $_POST['username'];
            $user->password = $_POST['password'];

            $userDao->save($user);
        }
    }
}

Hope this helps.

Martin Bean
  • 38,379
  • 25
  • 128
  • 201
  • -1 : none of that code has place in controller. You have domain business logic in presentation layer. And you `User` "class" provides no encapsulation or behavior. – tereško Sep 12 '12 at 20:17
  • Oh .. and your code example of `UsersDAO` actually is a data mapper, [not a DAO](http://java.sun.com/blueprints/corej2eepatterns/Patterns/DataAccessObject.html). DAOs do not interact with storage directly – tereško Sep 12 '12 at 20:42
  • @tereško Feel free to post your solution as an answer. Also, not sure how you can say I have domain business logic in the presentation layer when I haven't even posted an example of the presentation layer. – Martin Bean Sep 13 '12 at 08:15
  • 1
    Controller is part of presentation layer FYI. – tereško Sep 13 '12 at 09:47
  • This is the right approach. Teresko is so concerned with conceptual abstraction theory that the solution he provides defies Object Oriented Programming. Rule #1 of OO - Use objects. – sg- Jul 11 '17 at 03:55