4
\$\begingroup\$

I'm trying to learn OOP with PHP (I know it's hard because It's not an OOP language). I'm rewriting my procedural code to OOP.
Please provide any feedback on my code, naming and file/folder structure.

Structure:

\classes  Procurement.php  ProcurementDetails.php\services  DatabaseService.php  ProcurementService.php  ProcurementsService.php\public  Index.php\routes  Procurements.php

classes\Procurement.php

<?phpclass Procurement {    public int $id;    public null | array | string $group;    public null | string $object_name;    public null | string $procurement_type;    public null | string | object $responsible_user;    public null | string $announcement_time;    public null | string $status;    public null | string | object $creator;    public function __construct()    {        if ($this->responsible_user) $this->responsible_user=json_decode($this->responsible_user);        if ($this->group) $this->group=json_decode($this->group);        if ($this->creator) $this->creator=json_decode($this->creator);    }    private function __set($name, $value) {}};

classes\ProcurementDetails.php

<?phprequire_once __DIR__ . '/Procurement.php';class ProcurementDetails extends Procurement {    public null | string $description;    public null | int $sum;    public null | string $procurement_method;    public null | string | array $contact_person;    public function __construct()    {        if ($this->contact_person) $this->contact_person=json_decode($this->contact_person);        parent::__construct();    }}

services\DatabaseService.php

<?phpinterface IDatabaseService {    /**     *     *     * @return PDO     */    public function GetPDO();}class DatabaseService implements IDatabaseService {     /**     * PDO object     *     * @var PDO     */    private $DB;    private $host   = '';    private $dbname = '';    private $user   = '';    private $pass   = '';        /**     * Creates the connection     *     * @return void     */    public function __construct () {        $this->DB = new PDO("mysql:host={$this->host};dbname={$this->dbname};charset=utf8mb4",$this->user,$this->pass);        $this->DB->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);        $this->DB->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);        $this->DB->setAttribute(PDO::ATTR_STRINGIFY_FETCHES, false);    }        public function getPDO() {        return $this->DB;    }    /**     * __destruct     *     * @return void     */    public function __destruct () {        $this->DB=null;    }}

services\ProcurementsService.php

<?phprequire_once __DIR__ . '/../classes/Procurement.php';require_once __DIR__ . '/ProcurementService.php';interface IProcurementsService {        /**     * Returns all procurements from database     * Procurement[]     *     * @return Procurement[]     */    public function GetAllProcurements();    /**     * GetById     *     * @param  int $id     * @return IProcurementService     */    public function GetById(int $id);}class ProcurementsService implements IProcurementsService {    private PDO $_db;        /**     * __construct     *     * @param  PDO $db     * @return void     */    public function __construct(PDO $db)    {        $this->_db = $db;    }        /**     * GetById     *     * @param  int $x     * @return IProcurementService     */    public function GetById(int $id): IProcurementService {        return new ProcurementService($id, $this->_db);    }        /**     * Returns all procurements from database     * Procurement[]     *     * @return Procurement[]     */    public function GetAllProcurements(): iterable {        $stmt=$this->_db->query("SELECT ....");        return $stmt->fetchAll(PDO::FETCH_CLASS, 'Procurement');    }}

classes\ProcurementService.php

<?phprequire_once __DIR__ . '/../classes/ProcurementDetails.php';interface IProcurementService {    /**     * Returns more info about the procurement     *     * @param  int $id     * @return ProcurementDetails     */    public function GetDetails();    public function GetFiles(); //Not implemented    public function GetComments(); //Not implemented}class ProcurementService implements IProcurementService {    private int $id;    private PDO $_db;    public function __construct(int $id, PDO $db)    {        $this->id = $id;        $this->_db = $db;    }    /**     * Returns more info about the procurement     *     * @param  int $id     * @return ProcurementDetails     */    public function GetDetails(): ProcurementDetails    {        $stmt = $this->_db->prepare("SELECT with details....");        $stmt->execute([$this->id]);        return $stmt->fetchObject('ProcurementDetails');    }}

routes\Procurements.php

<?phpuse Psr\Http\Message\ResponseInterface as Response;use Psr\Http\Message\ServerRequestInterface as Request;require_once __DIR__ . '/../services/DatabaseService.php';require_once __DIR__ . '/../services/ProcurementsService.php';$app->get('Procurements', function(Request $request, Response $response) {    $DB=new DatabaseService();    $ProcurementsService = new ProcurementsService($DB->getPDO());    $response->getBody()->write(json_encode($ProcurementsService->GetById(100)->GetDetails()));    return $response->withHeader('content-type','application/json')->withStatus(200);});

I felt like I got off track with OOP, so I don't have much code.BTW! I am using PHPSLIM.

askedOct 17, 2021 at 15:56
flowstacker's user avatar
\$\endgroup\$
6
  • 4
    \$\begingroup\$If you down vote a question please leave a comment explaining why you down voted the question. I don't see anything here that suggests there should be a down vote.\$\endgroup\$CommentedOct 17, 2021 at 22:30
  • \$\begingroup\$Just one quick note. The "classes" directory would better be called "entities".\$\endgroup\$CommentedOct 18, 2021 at 5:05
  • \$\begingroup\$Thanks @pacmaninbw , was about to delete the post because of the downvote... slepic Ok, that makes more sense.\$\endgroup\$CommentedOct 18, 2021 at 5:46
  • \$\begingroup\$The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title tosimply state the task accomplished by the code. Please seeHow do I ask a good question?.\$\endgroup\$CommentedOct 18, 2021 at 6:59
  • \$\begingroup\$I didn't downvote, but I could understand how someone would downvote simply due to the title not following our guidelines. Moreover, the question's body also doesn't explain what the code does.\$\endgroup\$CommentedOct 18, 2021 at 7:00

1 Answer1

4
\$\begingroup\$

Firstly, PHP is 100% an 'OOP' language. It provides you with all the necessary functionality to conform to all of theOOP principles.

Secondly, you should definitely familiarise yourself with thePHP Standards Recommendations (PSR) as they will guide you in keeping everything inline with the standard conventions that PHP developersshould follow when using the language.

Ok, with those nitpicking points out of the way, let's have a quick look at your code.

Utilisation of Interfaces

It's good to see you have the foresight to use interfaces, but your utilisation of them is a little off. One of the benefits of using interfaces is to ensure anything that implements it, conforms to the contract it has defined. This allows us to write a piece of code and swap out the implementation of the interface with something else, knowing full well that we won't need to change any of the utilising code.

Example:

We declare an interface for connecting to our database. As we may decide to implement different types of database connections, we need to selectively identify functions that will be needed for all database implementations.Note: I've kept it simple to ensure you grasp the concept, as opposed to what It's actually doing...

<?phpnamespace App\EXAMPLE;interface DatabaseInterface{    public function connect();    public function disconnect();    //Any other functions}

This interface allows us to create different types of database connections and ensures the code we write to connect to the database, will remain untouched when swapping it out.

Here's what an implementation may look like utilising PDO.

<?phpnamespace App\EXAMPLE;class PDODatabase implements DatabaseInterface{    public function __construct(/* Any relevant params */)    {    }    public function connect()    {        //Connect using PDO    }    public function disconnect()    {        //Disconnect    }}

Now in the area where we connect to the database, we can use the interface to define the type and then use any of the interface functions.

<?phpnamespace App\EXAMPLE;class DatabaseService{    private DatabaseInterface $database;        public function __construct(DatabaseInterface $database)    {        $this->database = $database;    }    public function setup()    {        //Any setup that needs to be done        $this->database->connect();    }}

When we define the type as an interface, it allows us to swap out the implementation with another. For example, say we wanted to change our PDODatabase to a MySQLi connection. We could easily write aMYSQLiDatabase.php class which implements theDatabaseInterface.php and pass that into ourDatabaseService.php constructor. And because any class that is passed into the constructmust conform to the interface, we know that there will be a connect and a disconnect function. This means we don't need to touch anything in theDatabaseService.php class because we have written code that requires an interface instead of a concrete implementation.

Avoid Ambiguity

I see that you're declaring your class variables with union types, which is PHP 8 and above, but just because you can doesn't mean you should.

public null | array | string $group;public null | string $object_name;public null | string $procurement_type;public null | string | object $responsible_user;

Imagine coming back to this code in 6-12 months time. Trying to debug this would be a nightmare because PHP is very forgiving when it comes to types. I would personally recommend to only one type with the allowance of null if it is required.

public ?string $group;

The same can be said for functions that return something. PHP developers are renowned for returning many different types (mixed) from functions. My personal opinion on this is that it should be avoided because it just over complicates everything later down the track.

If you want to further your knowledge on building extendable code, I would highly recommend researching theSOLID Principles. There are a lot of great resources out there to help you along the way. Start off small and get a good understanding of everything before getting too adventurous and biting off more than you can chew.

answeredOct 18, 2021 at 5:50
Savlon's user avatar
\$\endgroup\$
6
  • \$\begingroup\$Thank you so much for the detailed answer! I tried to recreate the databaseservice, but what would I pass to the ProcurementService? Previously I passed the PDO object (connection).\$\endgroup\$CommentedOct 18, 2021 at 6:29
  • \$\begingroup\$That’s where you pass an interface as opposed to a concrete implementation. Code to interfaces, not concrete classes. PDO is a concrete implementation. This means that if you ever need to swap it out for something else, you’ll need to rewrite your ProcurementService class.\$\endgroup\$CommentedOct 18, 2021 at 7:02
  • \$\begingroup\$So in the ProcurementService class I cannot access PDO directly? So I should create a query function in the DatabaseService class?\$\endgroup\$CommentedOct 18, 2021 at 7:21
  • 1
    \$\begingroup\$Yeh why not give that a go. If you are using this to actually create something though, I’d suggest using some kind of framework instead because it’s much cleaner and you can focus on the actual program instead of designing your own core functionality. Designing your own framework is biting off more than you can chew in my opinion. You will learn very quickly why the structure and adhering to SOLID principles is important. If you use someone else’s framework, all the complex/advanced stuff has been thought of and implemented already. If you’re doing this to learn then go for it\$\endgroup\$CommentedOct 18, 2021 at 7:26
  • 1
    \$\begingroup\$There are old versions of PHP that I would not try to write OOP in.\$\endgroup\$CommentedOct 18, 2021 at 12:38

You mustlog in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.