3

If you are using an mvc to build a user profile, would it be better to have the conditional statements used to work out the display type of the comments within a function in the model or in the controller like this:

For example I have 3 classes

  • Comments
  • Member
  • Admin (extends member)

Some example make use of Pseudo code where functions are missing

Option 1

Dependant of the type of user logged in the showComments function returning the comments would give back different information.

class user {

    function isLoggedIn() { //Check if a user is logged in }

    function getUserType() { // return user type }

    function showComments($id) { //comments code }
}

class admin extends user {
    function showComments($id) { //comments code }
}

Then use code within the controller to determine dependant upon the user type logged in which to show?

$profileContent = $user->getOtherContent();

if ($user->isLoggedIn() && $user->getUserType() == "member") {
    $member = new member();
    $comments = $member->showComments($profileId);
}
elseif ($user->isLoggedIn() && $user->getUserType() == "admin") {
    $admin = new admin();
    $comments = $admin->showComments($profileId);
}
else
    $comments = $user->showComments($profileId);

require 'templates/profile.php';

Option 2

As this is a custom framework I could move everything into a function within the model and have one function in user to check the comment type to display:

abstract class user {

    function isLoggedIn() { //Check if a user is logged in }

    function getUserType() { // return user type }

}

class profile {

    function showComments($profileId, $user) {

        if (User::isLoggedIn() && User::getUserType() == "member") {
            $comments = //database query and formatting for member
        }
        elseif (User::isLoggedIn() && User::getUserType() == "admin") {
            $comments = //database query and formatting for admin
        }
        else
           $comments = //database query and formatting for guest

        return $comments;
    }
}

Using a controller like:

$profile = new profile($profileId);
$comments = $profile->showComments();

require 'templates/profile.php';
hakre
  • 193,403
  • 52
  • 435
  • 836
Dan
  • 11,914
  • 14
  • 49
  • 112
  • Are you using a particular framework? You mention you have a model and controller, but what about a view? – Kekoa Jan 11 '12 at 18:37
  • whoops I missed that part out, the view is required after the if/else in the controller. The comments are passed to it as `$comments` – Dan Jan 11 '12 at 18:41
  • There is no rule for what you ask for, so do whatever does best for you. – hakre Jan 11 '12 at 18:43
  • Hi @Silver89! The issue is with the code that displays the comments view depending the type of the user, or with the code to get data from the DB for each type of user? – nick2083 Jan 15 '12 at 22:22

5 Answers5

5

Technically either is correct. The MVC pattern is intentionally abstract, and there is some debate about what the proper domain of the model vs. the controller are.

There is probably a "better" answer depending on the exact framework you are using. Otherwise, do what you think makes most sense.

Update - In light of changes to your question i'd like to tailor my answer a bit:

For Option 1 it would make more sense to design the model like so:

class user {
    function isLoggedIn() {}
    function getUserType() {}
    function showComments() {}
}

class admin extends user {
    function getUserType() {}
    function showComments() {}
}

class member extends user {
    function getUserType() {}
    function showComments() {}
}

In the controller, $user should be instantiated as a admin, member or user (this could be done with a static factory or in the controller directly). After that your controller is simply

if ($user->isLoggedIn()) {
    $comments = $user->showComments($profileId);
}

(To make this even smarter, profileId could be set as a class property (unless users have multiple profiles?)

Option 2, otoh, is a smart use of model-to-model design.

The only real difference I see is how you conceptualize Comments. Do you think of them as a part of the User (with weak ties to the Profile)? or a part of the Profile (with weak ties to the User)? Neither approach has any particular pain points that I see off the bat, so the best option is to run with the one that makes the most sense to you.

Mr Griever
  • 4,014
  • 3
  • 23
  • 41
  • 2
    +1 framework matters as they usually tend to coalesce opinions into their own "best practices" – Kekoa Jan 11 '12 at 18:46
2

I believe that this belongs in the Model. I feel that user authentication and validation is something that does not belong in a Controller, ultimately it is working with and validating data and in MVC that is done in the Model.

I don't think this is a good fit in the Controller for another reason. It's making it too smart. The Controller should only know that it needs to provide some comments to the View. Why does the Controller need to know what type of User is logged in? It shouldn't, it should only know the data that the Model says it should have.

Charles Sprayberry
  • 7,741
  • 3
  • 41
  • 50
2

I try to move any logic that handles data (retrieval, manipulation etc) to the model. That sometimes includes conditional statements.

Personally, I would make "showComments" and instance method for the User class but it's really a matter of opinion (also known as "best practice" by people with strong opinions).

I tend to adopt the Fat Model, Skinny Controller approach when it seems prudent.

Community
  • 1
  • 1
B. Notess
  • 525
  • 1
  • 6
  • 11
  • how would showComments know which type of user it was though if it was done like that? I'm kind of stuck as to coding it other ways because my minds now set on this approach and I can't see it from another direction which would suite it better? – Dan Jan 11 '12 at 19:02
  • It depends what framework you are using. If you are instantiating a user from a database record, the user should have an id associated with it. Something like $userComments = findComments($this->ID); Hope that makes sense (probably not). – B. Notess Jan 11 '12 at 19:07
  • I have updated the question, option 2 now shows more what I think you mean? – Dan Jan 11 '12 at 19:28
  • @Silver89 That's closer to what I would do. I'm not sure why an instance method in the Profile class needs it's own ID ($profileId) passed as an argument. – B. Notess Jan 11 '12 at 19:36
  • that's a good point, is it possible to pass the object $user to the profile showComments function to verify the userType? If I try and initiate a new user in showComments I get `Cannot instantiate abstract class User` – Dan Jan 11 '12 at 20:02
2

I guess it all comes to your interpretation of what a model / view / controller stands for. Here's my 2 cents:

Model

A model should handle one and only one application logic. If you have a User model, then inside that model there should be only methods and attributes relating to User. That said, if possible, try not to have Comments logic inside the User model. They belong in the Comments model. Making conditionals that call another model inside the current model would be a crossover to another part of the application. That's what controllers are for.

Models should always respond with abstract structures. A model should never respond with something formatted, or string based. It should be usually an array, an object, a simple int (preferably a model constant) or a boolean.

Controller

The controller should handle input data (even a plain webpage view is an input data) and based on that data, it should call for an application logic.

In your controller you should have all conditionals relating to calling one or another model.

Based on the abstract result, it should either call another model, or gather the response and merge it so it can be passed on to a view as simple structures (try not to pass objects to the view, but if you are faced with complex structures as a model return value, gather them into an array and pass that to the view).

View

The view is the most straight forward of the three. It should have formatted response with simple instructions which just handle the display (or in some cases the serialization or transformation of the controllers response into something viewable by another application - browser, or a 3rd app calling an api)

It all comes down to your gasp of the MVC architecture. Try to keep a constant development "strategy" throughout your project.

Hope it helps

capi
  • 1,453
  • 12
  • 10
1

Business logic belongs in controllers. Data logic belongs in models.

So, if you need to show certain data depending on user input, then that belongs in the controller. However, actually getting/storing the data belongs in the model.

Xeoncross
  • 55,620
  • 80
  • 262
  • 364
  • So there is no difference between having a call to one build class to get all the data needed for that page than there would be to call each class from the controller to get the same data? Just the controller would have more code with the second approach? – Dan Jan 14 '12 at 22:54