Concepte in OOP si Refactoring

Pentru a intelege mai bine conceptele din spatele OOP, am creat un exemplu foarte simplu, o simpla implementare a modelului arhitectural MVC, implementare care in acest moment nu respecta nici un principiu in programare.Ideea este sa inteleg si sa intelegem impreuna cum acest exemplu poate fi rescris mult mai bine, ce probleme implica in acest moment, ce probleme pot aparea in viitor, in caz ca proiectul evolueaza, respectand principiile in programare, ce beneficii avem pe termen scurt dar si pe termin lung etc.Cu alte cuvinte, discutam despre un proces de Refactoring.

Structura directoarelor:

Proiect
App
Controllers
-UserController.php
Models
-User.php
System
Database
-MysqlDatabaseConnection.php
index.php

Fisierele noastre:

index.php

<?php

require 'app/Controllers/UserController.php';
require 'app/Models/User.php';
require 'system/Database/MysqlDatabaseConnection.php';

// let's assume this data was sent through a POST request
$credentials = [
    'username' => 'John Doe',
    'email'    => '[email protected]',
    'password' => '123'
];


$userController = new App\Controllers\UserController();

// pass the credentials to the method
$userController->processSignup($credentials);

UserController.php

<?php namespace App\Controllers;

use App\Models\User;

class UserController
{
	private $user;

	public function __construct()
	{
		$this->user = new User();
	}

	public function getSignup()
	{
		// render the signup page
	}

	public function processSignup($data)
	{
		// validation
		if (empty($data['username']) || empty($data['email']) || empty($data['password'])) {
			// redirect to the singup page with a message
		}
		
		// create a new user using the User model
		$this->user->create($data);

		// redirect to the signup page with a success message
		
		
	}
}

User.php

<?php namespace App\Models;

use System\Database\MysqlDatabaseConnection;

class User
{
	protected $db;
	protected $table = 'users';

	public function __construct()
	{
		$this->db = new MysqlDatabaseConnection('localhost', 'root', '', 'test');
	}

	public function create($data)
	{
		// use the database connection and create a new row of records in database
	}
}

MysqlDatabaseConnection.php

<?php namespace System\Database;

class MysqlDatabaseConnection
{
	public function __construct($host, $username, $password, $dbname)
	{
		// prepare database connection
	}
}

Cred ca acest exemplu este destul de explicit.

Unde ai auzit tu ca un constructor ar returna ceva?

2 Likes

Apropo, este creat special pentru incepatori, stiu si eu prea bine pentru ce este folosit un constructor.

Pai ori e creat pentru incepatori, caz in care ar trebui sa corectezi comentariile, astfel incat acestia sa nu invete tampenii, ori e creat pentru tine si “tu stii”.

2 Likes

Foarte bun punctul tau de vedere, dar totusi eu am avut alta intentie, in fine.

Dupa ce ai corectat asta, corecteaza si cel putin crearea conexiunii mysql in model. Acea conexiune o vei crea in index si o vei injecta via constructors.

(de fapt, conexiunea nu are ce sa caute in model, dar merg pe conceptia ta)

Nu cred ca ai inteles ceva din ce am vrut sa spun eu, exemplul este facut asa, astfel incat sa vina persoane ca tine, de aceea ne aflam aici, sa argumenteze de ce nu este bine si cum trebuie rescrris.

Conexiunea trebuie extrasa de acolo si injectata din exterior deoarece vrei sa o refolosesti in diferite modele, nu sa creezi cate o conexiune pentru fiecare model. In timpul unei singure cereri HTTP poti jongla cu mai multe modele.

Poti sa argumentezi putin?
Macar o bucata de cod? Adica eu inteleg ca tu zici de ceva de genul.

index.php

...
$connection = new PDO($dsn, $user, $password);
$user = new User($connection);
$alt_model = new AltModel($connection);
...

Ce legatura are codul cu argumentatia? [2] Argumentatia ti-am dat-o deja. Un alt argument este “respectarea D-ului din SOLID”. Pentru intreaga argumentatie gasesti pe web articole si carti despre SOLID si D-ul din SOLID.

[quote=“emanuel, post:9, topic:2383”]
Adica eu inteleg ca tu zici de ceva de genul.
[/quote] Eu zic ceva asa:

$connection = new PDO($dsn, $user, $password);
$userController = new App\Controllers\UserController($connection);
$userController->processSignup($credentials);
//...
$otherController = new App\Controllers\OtherController($connection);
$otherController->otherAction();

In viata reala, in fix aceasta situatie, ai folosi un service locator pentru a ajunge la acea conexiune din interiorul controllerului, sau, in functie de hype-ul unui framework folosit, acel service locator ar fi ascuns sub numele de “DiC”.

Acel DiC sau service locator ar fi injectat via ctor, si nu doar conexiunea - pentru ca intr-o aplicatie mai complexa ai mai multe servicii, nu doar baza de date. Si in general, framework-ul ar face aceasta injectare pentru tine, iar tu in controller iti iei serviciul dorit:

try {
    $connection = $this->serviceLocator()->findService('db');
}

E din punctul de vedere al discutiei noastre aceeasi arhitectura [1], insa cu un nivel de abstractizare intre - ai acel service locator la mijloc.

Dupa cum vezi, ai un try acolo, ceea ce inseamna cod mai fragil. Un service locator sau un DiC iti face codul mai fragil, e unul dintre dezavantajele unei astfel de tehnici.

Din acest motiv (si altele) nu sunt fan DiC in codul care conteaza (modelele, in modele e business value), dar il accept in controller deoarece oricum ce e in afara modelelor e un cod in care “nu am incredere”, e doar glue code plin de vendori si tot acel glue code de tot rahatul.

Pastreaza-ti modelul vendor-free.

Revenind asupra codului

try {
    $connection = $this->serviceLocator()->findService('db');
}

o variatie ceva mai curata este mutarea acestui cod intr-o metoda privata sau protected din controller, si vei avea:

$connection = $this->databaseConnection();

(cu sau fara try de jur imprejur, depinde cum faci error handling in acea metoda helper databaseConnection). E mai curata din cateva motive:

  • izolarea mai buna (mai buna, nu perfecta) a codului fragil
  • linia de cod are deja doua nivele de indirectie (ai de doua ori -> in acel cod $this->serviceLocator()->findService('db');), ceea ce incepe sa miroasa a violarea legii lui Demeter. Extragand acea linie intr-o metoda separata, macar izolezi mirosul
  • poti adăuga type hinting, deci IDE-ul poate face autocomplete

[1] “injectezi ceva, si acel ceva iti ofera acces la resursa dorita”.

[2] adica daca eu iti dau codul, cum ajungi tu de la cod la argumentatie? Relatia cod ↔ argumentatie nu e bijectiva. Poti cel mult sa iti imaginezi o argumentatie, insa nu vei sti daca ai gasit aceeasi argumentatie pe care as fi dat-o eu.

1 Like

De ce stie controller-ul despre conexiune? Just asking. Nu mai spun ca instantierea unui controller e facut de framework.

Deoarece el e numitorul comun, care coordoneaza celelalte activitati.

Mi se pare buna ideea de a nu folosi un framework in codul de pe acest topic, deoarece astfel putem analiza mai obiectiv ceea ce ar face un framework si putem intelege si argumenta notiuni de arhitectura cu mai multa claritate.

1 Like

O fi comun, dar la fel ar fi si un obiect Request sau Response, nu? Daca apoi ar mai avea si altceva in comun in logica lor, ai injecta si aia in ambele controller-e? Ideea mea este ca, a fi comun nu inseamna ca este responsabilitatea controller-ului sa expuna conexiunea.

Tu vorbesti in termeni de obiecte - unde nu-i rau sa injectezi dependinte -, dar ar trebui sa vorbesti in termeni de MVC - unde responsabilitatea difera. Si cuplarea unui controller de o conexiune DB nu e ceva ce vad des. Cand ai spus “conexiunea nu are ce sa caute in model”, sper ca nu te refereai s-o muti direct in controller, ci intr-un UserRepository ceva, nu?

1 Like

Foarte bun pontul cu UserRepository, in contextul actual, UserController este considerat client, cu alte cuvinte, nu-l intereseaza cum primeste aceste date de la model, el trebuie doar sa stie cum sa folosesca comanda pentru a primi aceste date.

Revin cu un update cu mai multe informatii despre acest subiect.

Pentru cei care nu cunosc termeni ca Service si Client, in cateva cuvinte, un obiect de tip service este un obiect creat pentru a face ceva anume( ofera conexiunea cu baza de date etc), in timp ce un obiect de tip client, este un obiect ce se foloseste ( depinde ) de service pentru a face ceva.Un obiect nu poate fi in acelasi timp si client si service, dar in contexte diferite poate fi service sau client.

Conexiunea DB in Request violeaza SRP. Un request sau response e doar un DTO care mai are in plus si particularitatile protocolului (“http” de exemplu).

Dupa cum am spus, as folosi DiC.

Atata timp cat nu infectez modelul cu DiC, se pot face tot felul de compromisuri.

Vorbim despre OOP. MVC sau nu e un detaliu. Principiile curate de arhitectura si design OOP sunt aceleasi. Nu exista un “conflict de interese” intre MVC si OOP. Codul MVC insusi e scris in OOP, nu? Thus same rules apply.

Doar coordoneaza, injectand mai departe conexiunea, nu o consuma.[quote=“nush, post:13, topic:2383”]
Cand ai spus “conexiunea nu are ce sa caute in model”, sper ca nu te refereai s-o muti direct in controller, ci intr-un UserRepository ceva, nu?
[/quote]
Da, prin “model” inteleg toata colectia de clase care implementeaza exclusiv business logic.

In model nu apar nume de vendori (ex. “mysql”), nume de protocoale (ex “http”), accese la fisiere (ex “fopen”). Astea sunt servicii.

UserRepository e o idee buna, dar acum vorbim despre situatii teoretice, incercand sa ne imaginam o arhitectura mult mai complicata fata de ce avem concret in fata.

Daca cineva vine cu codul refactored cum am zis, o sa ma mai gandesc la ce alte lucruri mai pot fi imbunatatite in ACEL COD concret, altfel cadem in plasa over-engineering-ului.

A, ok, credeam ca vorbim despre organizarea OOP in arhitectura MVC, unde exista si alte principii.

PS: N-am zis sa bagi DB in Request, ci Request-ul in controller, pe langa DB si Response.

Nu am vazut niciun Request sau Response in codul initial.

Titlul spune refactoring, refactoring se face pornind de la ce ai, nu de la ce iti imaginezi ca ai avea. E adevarat ca poti introduce structuri noi… dar cine introduce in cadrul unui refactoring un intreg framework? Ar fi prea mult, ar fi over-engineering.

Codul din postarea #1 nu contine niciun framework, niciun protocol, nimic.

Repet: Daca cineva vine cu codul refactored cum am zis, o sa ma mai gandesc la ce alte lucruri mai pot fi imbunatatite in ACEL COD concret, altfel cadem in plasa over-engineering-ului.

Mi-am mai nuantat raspunsul.

Nu exista cine stie ce “alte principii” majore. MVC doar defineste responsabilitatile, nu face mai mult. Apoi se aplica aceleasi principii OOP deja cunoscute.

A, ca unii scriu carti in care iti prezinta MVC de ca si cum ar fi ceva magic, cand de fapt ei de fapt doar aplica principii OOP in contextul definitiei responsabilitatilor lui M, V si C, asta e marketing, nu arhitectura.

Adica: pentru orice principii mi-ai spune tu care crezi ca sunt “principii MVC”, eu pot sa iti numesc principiile OOP care stau de fapt la baza acelor “principii MVC”. Bine, presupunand ca nu vii cu aberatii, ci cu principii sanatoase.

a) Refactoring nu inseamna sa nu creezi noi clase sau servicii. Dimpotriva, deseori se intampla.

b) Avand de fapt un MVC, dar facand refactoring ca si cand n-ar exista, sugerezi restructurari nepotrivite. Inteleg ca vrei sa te rezumi doar la codul existent, dar il rupi din context. Or, cel putin in cazul de fata, contextul (faptul ca-i MVC si controller) dicteaza o cu totul alta abordare. Parerea-mi.

c) Stiindu-te programator defensiv, ma astept ca intotdeauna sa programezi “imaginandu-ti ce poti avea”.

Aka

  • nu as cupla controller-ul cu DB;

  • as crea un UserRepository - numai el stie de DB (crud, in speta)

  • as lasa Modelul user gol, aproape DTO, poate doar validare

    use App\Models\User;

    class UserController
    {
    private $userRepo;

      public function __construct()
      {
          $this->userRepo = new UserRepository();
      }
    
      public function signupAction()
      {
          $user = User::fromArray(Request::getParam('user_data'));
          $this->userRepo->save($user);
      } 
    

    }

Nu ma omor dupa User::fromArray(), dar se poarta pe la mine la munca. Nu prea-mi plac staticele, is all.

PS: Nu spun ca OOP nu are legatura cu MVC. Ambele de ex se ocupa cu separarea responsabilitatilor; adica un OOP bun favorizeaza MVC etc. Diferenta este ca MVC-ul are si alte concepte: OOP nu stie ce-i ala controller, de ex. In context OOP, un obiect DB (reprezentand conexiunea) injectat in ctorul unei clase e sexy, dar in MVC, cuplarea celei conexiuni direct in controller nu e. Acum nu stiu… Eu pot face neglija faptul ca-i MVC, dar poate mai vine cineva si vede DB in controller si spune ca-i bine. Care-i dificultatea daca-l consideram full MVC? Daca, precum spuneai, facem tot OOP?

Nu stiu de ce ai impresia ca as fi sustinut altceva. Nu avem pareri divergente.

Imaginatie imaginatie, nimeni nu zice nu. Dar depinde cat de departe mergi. Dupa cum am scris deja, eu nu as merge atat de departe incat in cadrul unui refactoring sa introduc 1. un intreg framework 2. un intreg protocol de comunicare (“http”). In refactoring ma limitez la a face structura extensibila pentru pasii urmatori, unii dintre pasi fiind poate un nou framework sau protocol de comunicare. Dar pasii insisi nu i-as face in timpul refactoringului. Pentru ca asta inseamna o duzina de teste noi (TDD) si o groaza de oportunitati de a pierde focusul si GTD.

Adica Value Object?

Modelul trebuie sa implementeze regulile de business.

Deci unde pui conexiunea DB astfel incat sa o refolosesti in toate celelalte repositories?

DB e un serviciu.

Foarte bine.