A bit of background, with some suggestions (in addition to the points tim mentioned):
- In the MVC pattern, the M (Model) isn't really something that can be poured into a single class. The Model part (with an upper-case M) is referred to as a Layer. If you write a data provider (a class that queries the DB), it's part of the Model layer. If you write a class that does computation or data processing (a service), it is part of the Model layer. If you write a class that simply contains data that belongs together (ie: an Id, the name and email of a user), this class is called a data model (with lower case m), and it, too, belongs in the Model layer.
- With this in mind, I'm afraid I have to say your class (
ContactModel) has a rather unfortunate name. I'd expect a ContactModel to be a data model. A container that uses getters and setters (which sanitize and validate the data), which I can pass around. I'd expect a CustomerDataProvider class or CustomerMapper class to be present to take care of the queries. The SELECT queries here should then return one or more CustomerModel instances
- Also important in the MVC pattern is to keep your controllers small. All a controller does, really, is to take the request parameters (user input), process it (pouring it into data models), and pass it on to the Model layer. This layer contains all of the actual logic. That's why it is also referred to as the business logic layer. This is the backbone of your application, it's where the magic happens.
So in basically, the traditional flow of an MVC application looks as follows:
[User Request]
||
|_==> Dispatcher (create Request object, figure out routing)
||
||<== dispatcher works out what controller and action to use
||
|_==> Controller extracts data from Request (creating data models)
/\ ||
// ||<== works out what part of the Model layer processes data
|| ||
returns || |===> [SERICVE] This is where it all happens
|| ||
|_=========_| <-- after service is done
What the service returns to the controller can then be passed on to another component of the Model layer (in case further computation is required), or the controller passes it on to the view.
Now the service layer is where all things important live. Suppose we're processing a login form. In that case, the controller action should look something like this:
public funciton loginAction(Request $req)
{
$form = new Form('loginFrm');//create the form
if ($req->isPost())
{//form was submitted
$form->processRequest($req);//the form processes the request
if ($form->isValid())
{//form submission was valid, meaning all fields were filled in
$service = new LoginService($config);//pass dependencies if applies
$login = $service->doLogin($form);
if ($login)
{//do the actual login in service
return $this->redirect('userMainPage', $login);
}
$form->setError('Invalid login');//set error message
}
else
{//Form was invalid
$form->setError('Not all required fields were filled');
}
}
//pass form to view, ready to be rendered
$this->renderView(
'login.twig',
[
'form' => $form
]
);
}
Take away the comments, and my rather spacious coding style, and you'll find that all of the code in my controller could be condensed to about 10 lines, without having to sacrifice readability too much. That's how it should be. However, let's look at this one line a bit more closely: $service->doLogin($form).
Like I said many times before already: everything that relies on external data (webservice calls, DB connectivity), or requires more complex computation does not belong in the controller. Hence, this simple looking service call will contain the most code.
About now, it might be good to mention the SOLID principles. More importantly the S part: Single Responsibility Principle. This very reasonable principle states (or rather dictates) that a class can have only one reason to change. That's a fancy way of saying that a class can have no more than one job. A class focusses on just one task: a data model's job is to contain data. That's all that it does. It can't connect to the DB, nor can it query for data. That's not its job. A DataProvider queries a database, but it doesn't directly connect to it: It might contain a PDO instance, which represents the connection, or it can be injected the config to establish this connection, but the connecting itself is done my PDO or mysqli. Just like these two PHP extensions don't occupy themselves with the actual result fetching (they return PDOStatement and mysqli_stmt instances to do that).
At this point, we've already mentioned 5 elements that make out our Model layer:
- Data models
- Service
- DataProvider
- DB connection (
PDO or mysqli)
- DB resultsets and queries (
PDOStatement, or mysqli_stmt)
Each of these components have a clear task to perform, and logic dictates that each task can go wrong. Therefore, each of these components must have means to notify the user that they were unable to do their job. If one of these components simply calls exit or die, you'd have the devil's own job of figuring out what component it is that fails, and why. Enter Exceptions. Let's examine what can go wrong, and how we can easily notify the user of this issue:
- Data models: if, for example, a
ContactModel is passed an invalid email address to its setEmail method, the model should pick up on this (using filter_var($argument, FILTER_VALIDATE_EMAIL)). If the email is invalid, the model should throw an InvalidArgumentException with a message like '"foo" is not a valid email address'
- The Service binds everything together, so it depends on there being a data provider, valid data and any number of other dependencies available. If one of them is missing, a service might throw a
RuntimeException instance, telling you (the user) it's missing a data provider it needs
- The DataProvider in turn requires a db connection to execute queries, if this is missing, it should throw another
RuntimeException. If you pass an instance of ContactModel to a method called findContactByEmail, but this instance doesn't have an email set, the dataprovider could throw a LogicException or a BadMethodCallException.
- The DB connections and statement/resultset objects will behave as you want, but it's common to have
PDO throw PDOException instances.
mysqli is a bit trickier. It's api is somewhat messy, so it's very common to see it wrapped in a class that checks for errors and throws a mysqli_sql_exception or mysqli_warning if something has gone wrong
So depending on what goes wrong, in what component, a different exception will be thrown. That makes your life a lot easier, because if you see a PDOException flying by, you simply know where to look.
So now, what does this LoginService::doLogin method look like? Well, this would be plausible:
/**
* Validates login form, returns full user model on success, false on failure
* @param Form $data
* @return UserModel|bool
* @throws RuntimeException
*/
public function doLogin(Form $data)
{
$user = new UserModel();
$user->setUserName($data->getLogin())
->setPassword($data->getPassword());
$provider = $this->getUserDataProvider();
if (!$provider)
{
throw new RuntimeException(
sprintf(
'%s did not receive the user dataprovider',
__CLASS__
)
);
}
if (!$provider->userExists($user))
return false;//user not found
$existing = $provider->getUserByLogin($user);//get full data
if ($existing->getHash() === $user->getHash())
return $existing;//return full data model
return false;
}
