MVC structured REST API in PHP

Question

This approach might be wrong or right, if you have something to add please correct me since i want to learn!

The past few weeks I'v been trying to build a MVC structure REST API in PHP.And this is my way of understanding how it works:

• So the User talks to the app thorough the Router.
• According to the Users request the router picks a controller if valid
• The controller picks the model and choses the right action to be done
• Than when the action is performed the model talks back to the controller
• The controller renders the output though the view

This approach might be wrong ,it's my opinion and my understanding though what I'v red

---

So the project it self

Since this will act as an API i have decided to put in a sub domain.For example api.domain.com

Here is the file & folder structure:

. ├── README.md ├── app │ └── MVCtest │ ├── controller │ │ └── test.php │ ├── core │ │ ├── conf │ │ │ └── db.con.yml │ │ ├── database │ │ │ ├── db.con.php │ │ │ └── db.handler.php │ │ ├── handler │ │ │ └── sql.error.php │ │ └── helper │ │ ├── jsonSerializer.php │ │ ├── model │ │ │ └── model.helper.php │ │ └── stdObject.php │ ├── model │ │ ├── Test │ │ │ ├── get.user.php │ │ │ ├── test.add.user.php │ │ │ └── update.user.php │ │ └── Test.php │ └── view │ └── api.json.php ├── composer.json ├── composer.lock ├── diagram_1.png └── public └── index.php 

• The public directory is the door to the web
• The app/core holds some helpers(Json Serialization) and handlers(error handler).Also not to forget it holds the database configuration and an abstract class that will have use in the model.
• The view hold JSON encoding and it's the end point
• The model hold the functions and communicates to the database
• The controller hold the controller and it's an extension to the router.

The thing that concerns me the most is everything :P But I would like if someone would check out my model. So basically the main mode for example Test.php ,hold functions for certain actions.It extends those actions from Test/classes Each of those classes holds a function and extends an class.

Here is it:

Test.php

namespace MVCtest\model; use MVCtest\core\database\DbHandler as DbHandler; use MVCtest\model\Test\AddUser as AddUser; use MVCtest\model\Test\GetUser as GetUser; use MVCtest\model\Test\UpdateUser as UpdateUser; class Test{ //Add User public function add_user($userObject){ $add_user = new AddUser(); return $add_user->main($userObject); } //Get User public function get_users($userObjects = []){ $get_user = new GetUser(); return $get_user->main($userObject = []); } //Update User public function update_user($userObjects){ $update_user = new UpdateUser(); return $update_user->main($userObjects); } } 

Here is one of the function classes:

namespace MVCtest\model\Test; use MVCtest\core\helper\model\ModelHelper as ModelHelper; class AddUser extends ModelHelper{ public function main($TestObject){ //Query $stmt = parent::$dbConn->prepare("INSERT INTO user(name,surname,age) VALUES(:name,:surname,:age)"); $stmt->bindParam(':name',$TestObject->name); $stmt->bindParam(':surname',$TestObject->surname); $stmt->bindParam(':age',$TestObject->age); return parent::query($stmt,NULL); } } 

And here are the classes the function classes on the model extend

ModelHelper.php

use MVCtest\core\database\DbHandler as DbHandler; use MVCtest\core\handler\SqlError as SqlError; class ModelHelper extends DbHandler{ public $TestObject; private $status = 200; private static $row = NULL; public function __construct($test = []){ $this->TestObject = $test; parent::__construct(); } public function query($stmt,$row = []){ try{ $stmt->execute(); $row != NULL ? self::$row = call_user_func(array($this,'fetchAll'), $stmt) : self::$row = NULL; }catch(\PDOException $e){ $error = new SqlError($e->getCode()); $this->status = $error->determiner(); } return [$this->status,self::$row]; } //Fetch All private function fetchAll($stmt){ return $stmt->fetchAll(\PDO::FETCH_OBJ); } } 

DbHandler.php

namespace MVCtest\core\database; use MVCtest\core\database\DbConn as DbConn; abstract class DbHandler { //Database Connection protected static $dbConn = null; public function __construct(){ $database = new DbConn; self::$dbConn = $database->setup(); } abstract function query($stmt,$row); } 

---

Here is the complete project o github

I would appreciate if someone would give a quick view on what i have built to tell me if it's at least okay.And what should be changed.

Answer 1

There seem to be a number of strange design and/or naming conventions that you are using here. It makes your intention hard to understand at times.

---

What are you building here? Are you building a framework or an application? Your file structure of most everything living in an app directory seems at odds with your decision to have some classes nested underneath labelled within a core library. If you have a core framework, should that not exist outside the construct of any given app that that leverages the framework?

I think you could benefit greatly in continuing to understand the proper use of Composer with regards to library/dependency management (including frameworks). If for nothing else, it would be useful to understand good patterns around structuring your frameworks/libraries/dependencies relative to applications that use these dependencies. You might ultimately find yourself building your framework as a totally separate codebase from your actual applications that use them. This is a proper separation of concerns to allow you to maximize re-use of the framework parts.

Additionally, you should really look at PSR-4 compliant structuring of your code to allow for autoloading. This prescribes a particular scheme for structuring your code and naming your class files. You are almost there, but your classnames are wrong.

---

Use meaningful names. This code is littered with examples of questionable naming choices, starting with the name of the package itself (MVCTest). Is this a framework for testing? Just because it may be experimental in your mind at this point doesn't mean it shouldn't be named according to what you intend it to be. Think about it. What have you named your Github repository (hint - the word test is nowhere to be found)? Why introduce this test nomenclature throughout your codebase (which could end up confusing a reader of your code or someone who might be considering using your Composer package)?

Why is the main application bootstrap class called Test? Why do you have models nested under a Test directory? This is now causing you to introduce Test into your namespaces even. This is bad naming. Name these things what they are.

---

Why would your main application class (Test as currently named) live in a Model directory? Don't confuse main application functionality with models used within the application.

---

I do not at all understand having separate classes like AddUser, GetUser, UpdateUser etc. These do no seem like models at all, but rather controller actions and/or methods that would live on a proper user model. Why do you need concrete instantiations of what amount to methods?

Would it not be better to have something like:

class UserFactory { protected static $pdo; public static function setPdo(PDO $pdo) { self::pdo = $pdo; } public static function getUser($id) { // return user object based on $id } public static function getUsers($data) { // return user objects based on criteria in $data } public static function addUser($userData, $pdo) { // create new user based on passed $userData and return user object. } } 

I don't understand how the update action would exist in a static context, which is, in essence, what your call pattern is (even though you are instantiating an object to make an update). I would think the user object to be updated would have already been marshalled based on one of the methods shown above and would simply have the means to update itself.

---

In your diagram, you talk about having a router, but I see no evidence of there being a router from either the code or directory structure you have shown. This is a critical piece of functionality for a framework. Where is it?

---

I am concerned that you have config files buried within your code base. These typically should not be part of your code package (other than maybe providing a template). For example, you should not be including DB credentials within your repository at all (and you absolutely should not be using root/root DB credential for an application EVER) (note to other readers, this was seen in GitHub repo, since code is not here). You have you DB credentials out on GitHub!!! Change them now and get this file out of your repo!

---

In your Test class. Your methods are unclear in intent, possibly due to poor parameter naming.

Why would a caller be passing a $userObject(s) to these methods and then have totally different objects instantiated inside the methods? What is happening here? Why would a get* method get objects passed to it when the intent is to get objects returned from it? Are you really passing ID's here (meaning perhaps the parameters should be renamed)?

---

You have an odd pattern of using these main() methods which seem like a paradigm from other languages you may have used in the past. main() has no special meaning in PHP. If there is code you want to be run upon object instantiation in PHP, that code needs to be in the constructor. Why make the caller set up objects in proper state? In other words, consider this using this sort of instantiation:

$object = new someObject($data); 

instead of:

$object = new someObject(); $object->main($data); 

---

With your DB-related classes, you seem to have fallen into a classic anti-pattern in working with DB objects in PHP. For some reason (probably because there are thousands of bad code examples out there on the web), almost every developer picking up PHP tries to write some DB abstraction or DB connection class. 99% of the time these classes add no value above and beyond working with the underlying PDO, Mysqli or similar classes.

In your case, what value do DBHandler and ModelHelper add? These classes present a number of problems:

• They don't provide meaningful abstraction above and beyond PDO (which is itself an abstraction).
• They don't provide any sort of connection management (as there is nothing here to stop a caller from spawning any number of DB connections).
• They don't provide any meaningful error recovery/handling (in fact they swallow exceptions, hiding error conditions from the caller which is likely better positioned to determine what to do in such conditions).
• They severely break encapsulation, in that you require a caller to instantiate and pass a PDOStatement object to the query() method.
• They wander off course of the Single Responsibility Principle, in that these classes are beginning to implement functionality that should either be in controllers (i.e. determining response codes) and/or models (like determining which rows from a database resource to retrieve).

I would honestly abandon these classes altogether in favor of having a true model base class (not a ModelHelper) which simply accepts a PDO object as a dependency. Do not do things like have model classes inherit from DB classes like you seem to be moving towards here when you start having your "model" classes in turn inherit from ModelHelper. DB connections and Users (for example) are two entirely different objects in your system and should not be part of the same class inheritance structure. Providing dependencies through inheritance is a poor plan. What happens for example when your model has a dependency on both a DB connection and a system logger? PHP does not support multiple inheritance, so you are out of luck. You really need to embrace the "composition over inheritance" mantra. Compose your dependencies into your classes rather than inherit them.

---

Why are you mixing snake_case and camelCase? I know PHP, as a language, is not really good about this (mainly from its legacy roots), but that doesn't mean libraries that you are writing should not be consistent amongst themselves with regards to the symbols you are defining.