Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

 
Reply to this topicStart new topic
> Prosta nakladka na PDO, wszelka krytyka mile widziana
q.michal
post 27.04.2017, 18:44:54
Post #1





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


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

http://wklej.org/hash/a36cc42ed55/
Go to the top of the page
+Quote Post
nospor
post 28.04.2017, 11:09:03
Post #2





Grupa: Moderatorzy
Postów: 36 429
Pomógł: 6289
Dołączył: 27.12.2004




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


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
q.michal
post 28.04.2017, 11:18:29
Post #3





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


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.

Ten post edytował q.michal 28.04.2017, 11:20:09
Go to the top of the page
+Quote Post
nospor
post 28.04.2017, 11:21:52
Post #4





Grupa: Moderatorzy
Postów: 36 429
Pomógł: 6289
Dołączył: 27.12.2004




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


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
q.michal
post 28.04.2017, 11:40:33
Post #5





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


Na szczescie niedlugo majowka, bedzie czas zeby to przepisac.
Go to the top of the page
+Quote Post
viking
post 28.04.2017, 11:44:40
Post #6





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1113
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


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.


--------------------
Go to the top of the page
+Quote Post
mrc
post 28.04.2017, 12:26:11
Post #7





Grupa: Zarejestrowani
Postów: 160
Pomógł: 27
Dołączył: 22.09.2008
Skąd: Tarnów

Ostrzeżenie: (0%)
-----


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.

Ten post edytował mrc 28.04.2017, 12:27:05


--------------------
Go to the top of the page
+Quote Post
Pyton_000
post 28.04.2017, 12:59:29
Post #8





Grupa: Zarejestrowani
Postów: 8 068
Pomógł: 1414
Dołączył: 26.10.2005

Ostrzeżenie: (0%)
-----


albo zwrócić pusty array jeśli zwracamy array. Choć exception jest lepszym pomysłem.
Go to the top of the page
+Quote Post
mrc
post 28.04.2017, 13:07:15
Post #9





Grupa: Zarejestrowani
Postów: 160
Pomógł: 27
Dołączył: 22.09.2008
Skąd: Tarnów

Ostrzeżenie: (0%)
-----


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ą).


--------------------
Go to the top of the page
+Quote Post
nospor
post 28.04.2017, 13:16:45
Post #10





Grupa: Moderatorzy
Postów: 36 429
Pomógł: 6289
Dołączył: 27.12.2004




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


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
com
post 29.04.2017, 19:20:50
Post #11





Grupa: Zarejestrowani
Postów: 3 032
Pomógł: 366
Dołączył: 24.05.2012

Ostrzeżenie: (0%)
-----


Wrzuć to na jakiegoś githuba i podstawowe pytanie jaki to ma cel?
Go to the top of the page
+Quote Post
q.michal
post 5.05.2017, 20:46:53
Post #12





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


hobbystyczno-edukacyjny smile.gif

v2: http://wklej.org/hash/6c51b2aeb3d/
Go to the top of the page
+Quote Post
viking
post 6.05.2017, 13:30:24
Post #13





Grupa: Zarejestrowani
Postów: 6 365
Pomógł: 1113
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


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


--------------------
Go to the top of the page
+Quote Post
q.michal
post 6.05.2017, 14:41:39
Post #14





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


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!
Go to the top of the page
+Quote Post
nospor
post 6.05.2017, 22:53:49
Post #15





Grupa: Moderatorzy
Postów: 36 429
Pomógł: 6289
Dołączył: 27.12.2004




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


--------------------

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
q.michal
post 8.05.2017, 19:08:25
Post #16





Grupa: Zarejestrowani
Postów: 111
Pomógł: 1
Dołączył: 24.12.2013

Ostrzeżenie: (0%)
-----


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() ?
Go to the top of the page
+Quote Post
Pyton_000
post 8.05.2017, 19:09:28
Post #17





Grupa: Zarejestrowani
Postów: 8 068
Pomógł: 1414
Dołączył: 26.10.2005

Ostrzeżenie: (0%)
-----


Dodaj komentarz do property z typem PDO.
Go to the top of the page
+Quote Post

Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 19.03.2024 - 07:49