Drukowana wersja tematu

Kliknij tu, aby zobaczyć temat w orginalnym formacie

Forum PHP.pl _ Oceny _ Prosta nakladka na PDO

Napisany przez: q.michal 27.04.2017, 18:44:54

Niestety wszystko sie tu nie zmiascilo, wiec zalaczam wrzutke do krytyki baaasmiley.gif

http://wklej.org/hash/a36cc42ed55/

Napisany przez: nospor 28.04.2017, 11:09:03

Klasa Statement nie ma najmniejszego sensu.
Przekazujesz do niej obiekt $pdo oraz statement i jedyne co ona robi to sprawdza czy jest polacznie pdo.... Hello, jesli masz juz statement to polaczenie bylo inaczej bys nie mial statement. wink.gif
Kolejna rzecza ktora robi to tylko wywolanie metod ala fetch() i na dodatek lapiesz tam wyjatki. metody fetch nie rzucaja wyjatkow, nie ma co lapac. Wyjatki sa na etapie prepare i execute co ty masz poza klasa.

Takze generalnie bez sensu jest tworzenie takiej klasy gdzie ona praktycznie nic nie robi a wiekszosc roboty jest zrobiona poza klasa. Reszty kodu nie przegladalem

Napisany przez: q.michal 28.04.2017, 11:18:29

Zamysl byl taki, aby po wykonaniu $db->connect mozna bylo od razu bezposrednio z tego obiektu odczytac dane, np: $db->fetchAllRows(); jak rowniez aby ta metoda zwraca obiekt klasy Statement z ktorego mozna byloby te dane odczytac pozniej:

  1. $s = $db->execute();
  2. $s->fetAllRows();


Jednak teraz rzeczywiscie wyglada to bezsensownie.

Napisany przez: nospor 28.04.2017, 11:21:52

Majac statement tez mozesz wywolac fetchAll.

Stworzyles klase Statement ktora nie robi nic innego jak jest posrednikiem do wywolania fetch() Kody sprawdzajace ktore sa w tych funkcjach w twojej klasie jak juz pisalem nie maja zadnego sensu

Napisany przez: q.michal 28.04.2017, 11:40:33

Na szczescie niedlugo majowka, bedzie czas zeby to przepisac.

Napisany przez: viking 28.04.2017, 11:44:40

Tylko po co przepisywać? To i tak się nie nadaje do użytku choćby dlatego że wpływasz na wyjątki. A projektów dużo lepiej napisanych jest pełno. Chyba że chcesz się czegoś nauczyć to ok.

Napisany przez: mrc 28.04.2017, 12:26:11

Zwracanie dwóch różnych typów w jednej metodzie jest złe. Od tego są wyjątki, aby je wyrzucać, kiedy funkcja nie potrafi zwrócić typu wartości, do którego została stworzona.

Napisany przez: Pyton_000 28.04.2017, 12:59:29

albo zwrócić pusty array jeśli zwracamy array. Choć exception jest lepszym pomysłem.

Napisany przez: mrc 28.04.2017, 13:07:15

Tutaj akurat bym się kłócił. Nie wiem o który przykład Ci chodzi, ale jeżeli szukasz czegoś w bazie, to pusty array nie jest taki zły (chociaż można lepiej, obiektem-kolekcją).

Napisany przez: nospor 28.04.2017, 13:16:45

@mrc kolegom chodzilo raczej nie o brak danych a o blad pobierania danych

Napisany przez: com 29.04.2017, 19:20:50

Wrzuć to na jakiegoś githuba i podstawowe pytanie jaki to ma cel?

Napisany przez: q.michal 5.05.2017, 20:46:53

hobbystyczno-edukacyjny smile.gif

v2: http://wklej.org/hash/6c51b2aeb3d/

Napisany przez: viking 6.05.2017, 13:30:24

A tak przy okazji. Wiesz że możesz dalej rzucić ten wyjątek?
catch (PDOException $e) {
throw $e;
}

Napisany przez: q.michal 6.05.2017, 14:41:39

Cytat(viking @ 6.05.2017, 14:30:24 ) *
A tak przy okazji. Wiesz że możesz dalej rzucić ten wyjątek?
catch (PDOException $e) {
throw $e;
}

Nie wiedzialem. Dzieki za hinta!

Napisany przez: nospor 6.05.2017, 22:53:49

if(!isset($this->pdo)) {
daj poprostu
if(!$this->pdo) {

if(!$this->stmt instanceof \PDOStatement) {
Nigdzie z zewnatrz nie ustawiasz stmt wiec albo t bedzie PDOStatement albo nic. Daj poprostu
if(!$this->stmt) {

No i jesli nie ma stmt to powinien leciec wyjatek a nie pusta tablica. Skoro ktos robi fetchAll a wczesniej nie zrobil execute to jest to blad

Gdy juz zamienisz to wyjatki to takie kody jak ten
if(!$this->stmt instanceof \PDOStatement) {
return false;
}
maja byc w oddzielnej funkcji i tylko wywolujesz te funkcje ktora rzuci wyjatkiem. nie ma sensu wszedzie duplikowac takich kodow

ta funkcja

  1. public function isConnected() {
  2. $this->clearError();
  3. if(!$this->pdo instanceof \PDO) {
  4. $this->setError('08003', 'SQLSTATE[08003] Connection does not exist');
  5. return false;
  6. }
  7. return true;
  8. }


powinna wygladac poprostu tak

  1. public function isConnected() {
  2. $this->clearError();
  3. return $this->pdo ? true: false;
  4. }
  5.  

Nie ma tam najmniejszego sensu ustawiac errora bo to nie zaden error

Napisany przez: q.michal 8.05.2017, 19:08:25

Przy okazji mam 1 pytanie niezwiazane do konca z tematem.
Mianowicie IDE wskazuje mi blad method not found w liniach w ktorych wywoluje jakas metode z PDO, np $this->pdo->commit();
Mozna to jakos obejsc? Powiedziec IDE ze tam jest PDO, jezeli w innej metodzie wykonuje $this->pdo = new \PDO() ?

Napisany przez: Pyton_000 8.05.2017, 19:09:28

Dodaj komentarz do property z typem PDO.

Powered by Invision Power Board (http://www.invisionboard.com)
© Invision Power Services (http://www.invisionpower.com)