2
\$\begingroup\$

I've read lots about getters and setters being evil, unless there is good call for one, but I cannot figure out how to implement that knowledge into my Model layer.

Say in my Model layer I have a class that has services like:

findStudentsByID($searchCriteria)
findStudentsByLastName($searchCriteria)

...

Do I require a getter or a setter, say $searchCritera, here so I can maintain Separation of Concerns?

Would it be wrong to simply query the model from the view with findStudentsByID(4);? Wouldn't that bind the two too much?

I thought I could use at least a setter so I can change the state of the class and then use a method.

For example,

API enforcement:

interrace IModel{
 public function setCriteria($searchCriteria);
 public function findStudentByID();
 public function findStudentByLastName();
}

In my model:

    class Model implements IModel{

    private $searchCriteria;

    public function setCriteria($searchCriteria){
      $this->searchCriteria = $searchCriteria;

    }

    public function findStudentByID(){
     some SQL "WHERE student_id = " $this->searchCriteria;
     return $results;
    }

    public function findStudentsByLastName(){
     some SQL "WHERE last_name = " $this->searchCriteria;
     return $results;
    }

    }

In my controller:

interface IController{
  public function someAction($searchCriteria);
}

class Controller implements IController{

    private $model;

    public function __construct(IModel $model) {
        $this->model = $model;
    }

//for argument say this is from a front controller passing the value
public function someAction($searchCriteria){

$this->model->setCriteria($searchCriteria);  //model state is changed

}

}

In my view:

interface IView{
  public function students();
}
class View implements IView{
  private $model;
  public function __construct(IModel $model){
    $this->model = $model;
  }

  public function students(){
    return $this->model->findStudentsByLastName();
    require_once("some_template_somewhere");  //get my template
  }

}

some template:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>Form Example</title>
  </head>
  <body>
  <h1>Students</h1>
    <p>
      <?php foreach($this->view->students() as $student) { ?>

            <?PHP echo $student["student_id"] ?>    </br>
            <?PHP echo $student["LAST"] ?>

    <?PHP } ?>

    </p>
  </body>

Implementation:

//some starting point:

$model = new Model();
$controller = new Controller($model);
$view = new View($model);

$controller->someAction($_GET['myAction']);  //changing model state;

$this->view->students();

Do I really need the view there? Would it be better to simply use this in a view/template (yes, I know they're different)?

  <?php foreach($this->view->model->findByStudentID(4) as $student) { ?>

        <?PHP echo $student["student_id"] ?>    </br>
        <?PHP echo $student["LAST"] ?>

<?PHP } ?>

The only thing I can see that will help with future maintainability, which is why I want my separation of concerns to begin with, is that I can switch the layers/concerns without breaking the other layers/concerns. The interface will make sure I have the setCriteria, etc.

From what I can see

  • I could switch out the model.
  • I could switch out the template.
  • I could switch out the controller.

But say someone makes their own Model. If they create an additional function, won't that break everything, rippling up? What if they implement only the stock functions in my model, but they return different information? I didn't really want a separate layer in my model layer, I thought I had abstracted myself to death already, though I feel like I might be slightly better off with abstract classes instead of interfaces.

EDIT: I changed the method being called to finding the students by last name so the example makes more sense.

\$\endgroup\$
  • \$\begingroup\$ Your view wants a list of students. Why isn't your model presenting that rather than a single student? \$\endgroup\$ – cHao Sep 4 '14 at 19:58
  • \$\begingroup\$ @cHao If I understand you correctly, I changed it to last name searching to things make more sense. \$\endgroup\$ – johnny Sep 4 '14 at 20:01
  • 3
    \$\begingroup\$ So why are you doing the search in the view? The view should be as ignorant as possible of where the data comes from. IMO it shouldn't even know there's a way to find students -- the controller and/or model should have set all this up already, and all the view should see is "Here's a list of students. Show them." \$\endgroup\$ – cHao Sep 4 '14 at 20:10
  • \$\begingroup\$ @cHao I'm using the template for that. I'm under the assumption of the View can talk to the model because the view is not just a template mentality. Maybe I don't understand it. \$\endgroup\$ – johnny Sep 4 '14 at 20:16
  • 3
    \$\begingroup\$ What you call IModel should be named StudentMapperInterface \$\endgroup\$ – Yang Sep 5 '14 at 2:42
3
\$\begingroup\$

Summary with answers below

MVC gone bad

I allready wrote an article on how MVC is abused, so I'm not going to repeat myself here.

So what is MVC?

MVC is a design pattern (or architectual pattern or even cake) that seperates our code into 3 distinct functionalities: Model, View and a Controller.

Model

The Model represents the Data. For instance a Student.

View

A view presents those Models. For instance in a table. It thus acts purely as representation layer.

Controller

The controller is the part that gets a command from the user/client and translates it to the correct internal command. e.g. "show user with id 4" => "pas the User Model with id 4 to the correct view"

Implementation

What MVC doesn't do is tell you anything about how you implement it. It also doesn't tell you how these different components interact.

MVC doesn't say anything about using other code. A lot people cram everything into MVC. So are you. Your Model is not a Model but a Repository (or Mapper or whatever you call it). Your Model acts as a layer to query your MySQL database that returns an object that represents your data.

Implementing the model:

the Student model will have method that will help you get information about the Model. For instance:

function getFullName() {
    return $this->firstName . ' ' . $this->lastName;
}

Retrieving the model

We ofcourse need a way of retrieving a Model. This could be done for instance using a Repository:

interface UserRepository
{
    public function find($id)

    public function getBy($key,$value)
}

and because we use Mysql, lets implement a repository that does all the fetching:

class MysqlUserRepository implements UserRepository
{
    public function find($id)
    {
        $result = $this->db->query('SELECT FROM users WHERE id = '.$id;
        return new User($result[0]);
    }

    public function getBy($key,$value) { ... }
}

This UserRepository could for instance be injected into the controller:

Implementing the controller

class UserController
{
    public function find($id)
    {
        $user = $this->repository->find($id);

        return ViewFactory::make('show.user', array('user'=>$user)));
    }
}

Here the UserController returns a View object. I opted for a Factory pattern approach here:

ViewFactory & view

class ViewFactory
{
    public static function make($viewName, $data) {
        $view = new View(VIEW_DIR . $viewName);
        $view->addData($data);
    }
}

And our view will look someting like this:

class View
{
    public function __construct($viewFile)
    {
        $this->viewFile = $viewFile;
    }

    public function addData($data)
    {
        $this->data = array_merge($data, $this->data);
    }

    public function render()
    {
        extract($this->data);
        include $this->viewFile;
    }
}

Implementing our view

Because I don't like writing $this->data->.. stuff in my templates, I use the extract function in the render function.

views/dir/show.user.tpl.php

<h1><?php echo $user->getFullName(); ?>
<p>Age: <?php print $user->getAge(); ?>

Just an Example.

Now this is just an example - and it should be treated as one.

Summary

to answer your questions

Do I require a getter or a setter, say $searchCritera, here so I can maintain Separation of Concerns?

No, why should searchCriteria be seperated? A searchCriteria needs context:

UserRepository::getById(4);

returns something completly different as

UserRepository::getOlderThenYears(4);

Would it be wrong to simply query the model from the view with findStudentsByID(4);? Wouldn't that bind the two too much?

YES, very wrong. A view is a presentational piece of software. It is stupid an knows nothing. Only how to present a certain object to the screen. Nothing more.

Models

Instead of using getters/setters on a Model. One could simply use public variables (or emulate them using the magic methods (PHP)). Since a Model is a representation of data, you could simply change it's values. After changes are made one could pass it back to the repository:

$user = Repository::find(4);
$user->name = 'foobar';
Repository::save($user);
\$\endgroup\$
  • \$\begingroup\$ On the model I was avoiding yet another layer of abstraction. The rest I have to study. Thanks for time. \$\endgroup\$ – johnny Sep 5 '14 at 13:07
  • 1
    \$\begingroup\$ @johnny Good modular OO code is all about abstracted responsibilities which results in low coupling and high cohesion. You should not try to avoid abstraction when it clearly would be the right thing to do. \$\endgroup\$ – Kid Diamond Sep 5 '14 at 13:18

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.