Is my design achieving Separation of Concerns in this MVC implementation?

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->findStudentsByLastNa

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

getFullName(); ?>
Age: 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);

StackExchange Code Review Q#61962, answer score: 4