Witam
Tworzę sobie klasę do zarządzania użytkownikiem, czyli dodanie, kasowanie, wyświetlenia loginu i póki co hasła, chciałem Was jako bardziej doświadczonych speców, zapytać czy wszystko jest dobrze w kodzie. Proszę również o ocenę podziału, zarządzania użytkownikiem na 2 klasy, z których każda robi co innego.

UserManager.php

[PHP] pobierz, plaintext 
<?php 
class UserManager{
 
	//Database connection
	public function __construct()
	{
	    try
	      {
	          $this->db = new PDO('mysql: host=localhost; dbname=test', 'root','');
	       }
	        catch(PDOException $e)
	       {
	           throw new Exception($e->getMessage());
	       }
	}
 
	//Create new user
	public function create($user, $pass)
	{
		$result=$this->db->prepare("INSERT INTO users VALUES('',:user,:pass)");
                $result->bindParam(':user',$user, PDO::PARAM_STR);
                $result->bindParam(':pass',$pass, PDO::PARAM_STR);
                   if(!$result->execute()){
                       throw new Exception('Konto nie zostało utworzone!');
                 }
	}
 
	//Delete user
	public function delete($id)
	{
		$result=$this->db->prepare("DELETE FROM users WHERE id=:id LIMIT 1");
                $result->bindParam(':id',$id, PDO::PARAM_INT);
                  if(!$result->execute()){
                       throw new Exception("Wystąpił bląd podczas kasowania użytkownika!");
                }
	}
}
?>
[PHP] pobierz, plaintext 

User.php

[PHP] pobierz, plaintext 
<?php 
class User{
 
	//Database connection
	public function __construct()
	{
	    try
	      {
	          $this->db = new PDO('mysql: host=localhost; dbname=test', 'root','');
	      }
	       catch(PDOException $e)
	       {
	           throw new Exception($e->getMessage());
	       }
	}
 
	//Select username
	public function getUsername($id)
	{
	 	$result=$this->db->prepare("SELECT user FROM users WHERE id=:id limit 1");
                $result->bindParam(':id',$id, PDO::PARAM_INT);
                   if(!$result->execute())
                    {
                        throw new Exception("Wystąpił bląd podczas pobierania użytkownika!");
                    }else{
 				$wynik=$result->fetch(PDO::FETCH_OBJ);
                               return $wynik->user;
                   }
     }
 
	//Select user password
	public function getPassword($id)
	{
	 	$result=$this->db->prepare("SELECT pass FROM users WHERE id=:id limit 1");
                $result->bindParam(':id',$id, PDO::PARAM_INT);
                     if(!$result->execute())
                       {
                           throw new Exception("Wystąpił bląd podczas pobierania użytkownika!");
                        }else{
                           $wynik=$result->fetch(PDO::FETCH_OBJ);
                           return $wynik->pass;
                      }
     }
}
?>
[PHP] pobierz, plaintext 

nie rozumiem dlaczego kazdy "user" samodzielnie laczy sie z baza i ma prywatne polaczenie
oprocz tego nie rozumiem czym sie rozni "manager" od "usera", skoro w obu przypadkach traktujesz te klasy tylko jako interfejs do laczenia sie z baza, a nie np. klasy dla samodzielnych obiektow konkretnych uzytkownikow

Z tą bazą akurat tymczasowo, chodzi mi o resztę kodu

Co do podziału User i UserManager, sugerowałem się wypowiedzą Crozin-a, z tego postu: http://forum.php.pl/index.php?s=&showt...st&p=880097 chyba, że go źle zrozumiałem.

Ty hasło trzymasz plaintekstem...?

md5 na główkę nie upadłem, żeby nie kodować go. Tyle, że kodowanie hasła jest poza klasą, i do klasy trafia zakodowane md5

raz dajesz echo raz exception przy try...
new pdo wywal po za klasy.

Spawnm echo poprawione I teraz pytanie do Was, lepiej jest robić taki podział jak widać u mnie, czy tworzyć jedną klasę, która utworzy, skasuje, wyświetli dane usera itp?

Jak dla mnie obie klasy powinny przyjmować jako parametr uchwyt do pdo - to raz.

Dwa klasa user - powinna w konstruktorze przyjmować id użytkownika jak dla mnie - przynajmniej wygląda, że byłoby lepiej.
getUsername - w obecnym przypadku powinno być raczej getUsernameByID

Wymodziłem coś takiego:

[PHP] pobierz, plaintext 
<?php 
class User{
	private $_db;
	//Database connection
	public function __construct(DB $_db)
	{
		$this->db=$_db;
	}
//Ciąg dalszy klasy User
[PHP] pobierz, plaintext 

Klasa DB

[PHP] pobierz, plaintext 
<?php
 
class DB
{
	public function __construct()
	{
		try
	   	{
	               $this->baza = new PDO('mysql: host=localhost; dbname=test', 'root','');
	   	}
	   	   catch(PDOException $e)
	   	{
	               throw new Exception($e->getMessage());
	   	}
	}
 
	public function prepare($query)
	{
		return $result=$this->baza->prepare($query);
	}
}
 
?>
[PHP] pobierz, plaintext 

I działa dobrze

Zainteresuj się frameworkami. Taka dobra rada.
Jak dajesz private/protected to rozpoczynaj nazwę od _ czyli private $_db;
Poczytaj o bindColumn() w pdo.
Jeśli chodzi o pytanie czy dzielić na kilka klas czy posiadać jedną której zadaniem będzie zarządzanie userami,
to już sam musisz zdecydować jak ci wygodniej.

Dziękuję za rady Co do frameworka, od 2 miesięcy używam ZF, ale chcę poznać OOP w lepszym stopniu niż podstawowy. Czasami tworzę jakiś bardzo mały projekt, gdzie framework nie jest kompletnie potrzebny, dlatego wolę mieć gotowy zestaw klas.

Wiem, że znając bardzo dobrze danego frameworka idzie bardzo szybko napisać aplikację, bo sam się o tym przekonałem.

//Kod poprawiłem

Witam,
troche odkurzę temat...

Przepraszam ze znow będzie pytanie "czy dobrze robie" ale jestem na etapie nauki OOP w PHP.

Moim zamiarem bylo stworzenie klasy która bedzie miala za zadanie skrócenie sobie pisania bloku "try {} catch {}".

Nie wiem czy to co wymodziłem w ogole się nadaje do zastosowania (ogólnie to mi działa tak jak założyłem) ale chcialbym sie dowiedzieć czy nie zrobiłem jakichś rażących błędów.

[PHP] pobierz, plaintext 
class DbManager extends PDO
{
	private $_dbh;
	private $_result;
 
 
	public function __construct()
	{
		try	{
			$this->_dbh = new PDO('mysql:dbname=rcs_wrs_dev; host=127.0.0.1', 'rcs_tomek', 'zmielonamarchewka',
			             array(PDO::MYSQL_ATTR_INIT_COMMAND => "SET NAMES utf8"));
			$this->_dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
		} catch( PDOException $e ) {
			echo( $e->getMessage() );
		}
	}
 
 
	public function fetch($query) {
		try	{
			if ( $_result = $this->_dbh->query($query)->fetch() )	{
				return $_result;
			} else {
				return 0;
			}
		} 
		catch (PDOException $e)	{
			echo( $e->getMessage() );
		}
	}
 
 
	public function fetchAll($query) {
		try	{
			if ( $_result = $this->_dbh->query($query)->fetchAll() ) {
				return $_result;
			} else {
				return 0;
			}
		} catch (PDOException $e) {
			echo( $e->getMessage() );
		}
	}
}
[PHP] pobierz, plaintext 

a używam tego tak:

[PHP] pobierz, plaintext 
$wiersz  = $db->fetch("SELECT Pressure FROM tasks WHERE task_id=94");
 
 
var_dump($wiersz);
 
// array(2) { ["Pressure"]=> string(4) "1029" [0]=> string(4) "1029" }
 
[PHP] pobierz, plaintext 

I wszystko wygląda jakby chciało działać a w razie błędu/literówki dostanę komunikat.

Dzieki za wszystkie uwagi.

Aby uzyskać podobny efekt wystarczy nie wywoływać poniższej linijki w konstruktorze i usunąć wszystkie instrukcje try catch.

[PHP] pobierz, plaintext 
$this->dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
[PHP] pobierz, plaintext 

Ten kod, który podałeś jest idiotyczny, wyrzuć go - nie żartuję. Jeśli się uczysz OOP to musisz się nauczyć stosować wyjątki i musisz zrozumieć ich przewagę nad tradycyjną obsługą błędów, która jest stosowana w całej bibliotece standardowej php ze względu na zaszłości historyczne.

OK, rozumiem że powinno używać się wyjątków (z tego co się orientuje to PDO ma swoje wyjątki i dlatego ich używam).
Poczytałem http://pl.wikibooks.org/wiki/PHP/Wyj%C4%85tki i tam piszą tak:


Nie rozumiem czemu mam się pozbyć bloku try, catch ? Nawet na php.net jest to stosowane: http://www.php.net/manual/pl/language.exceptions.php


Albo coś pominąłem albo nie mam zielonego pojęcia co ja robię tu

Usuwając bloki try catch miałem na myśli metody klasy DbManager. Bloki try catch powinny być w kodzie klienta obiektu DbManager. Twój kod jest bardzo zły bo nie mamy kontroli nad tym czy wystąpił błąd. Wywołując poszczególne metody nie wiemy czy nie spowodowała ona błędu. Przykładowo:

[PHP] pobierz, plaintext 
$db = new DbManager();
//nie wiemy czy zostało nawiązane poprawne połączenie z bazą danych, czy nie
 
$data = $db->fetch(...);
 
//nie wiemy czy wystąpił błąd, nie wiemy nawet czy $db ma poprawny stan (połączenie z bazą się powiodło)
//rób coś z $data
 
$data = $db->fetchAll(...);
 
//nie wiemy czy wystąpił błąd...
//rób coś z $data
[PHP] pobierz, plaintext 

Można poprawić ten kod poprzez zwracanie jakieś określonej wartości w przypadku błędu, zazwyczaj jest to wartość false lub -1 (czasy C oraz php przed pojawieniem się wyjątków )

[PHP] pobierz, plaintext 
//metody DbManager
 
public function fetch($query) {
  if(błąd) {
    return false;
  } else {
    return wynik zapytania;
  }
}
 
public function fetchAll($query) {
  if(błąd) {
    return false;
  } else {
    return wynik zapytania;
  }
}
 
//kod klienta
$db = new DbManager();
//nadal nie wiemy czy połączenie się powiodło
 
$data = $db->fetch(...);
 
if($data === false) {
  //obsługa błędu
} else {
  //rób coś z danymi
}
 
$data = $db->fetchAll(...);
 
if($data === false) {
  //obsługa błędu
} else {
  //rób coś z danymi
}
[PHP] pobierz, plaintext 

To rozwiązanie już jest odrobinę lepsze, ale minusem jest to że kod obsługi błędów jest wymieszany z kodem prawidłowego przebieg programu, błąd obsługiwać trzeba natychmiast po wywołaniu metody oraz jest pełno instrukcji warunkowych i powtórzeń kodu obsługi błędów.

Ostatnie rozwiązanie to wyjątki, jeśli wystąpił błąd to rzucaj wyjątkiem.

[PHP] pobierz, plaintext 
//przechwytuje wyjątki w kliencie obiektu DbManager, a nie w samej klasie DbManager
try {
  //w tym bloku nie zawracamy sobie dupy błędamy, bo zakładam że każda metoda wyrzuci wyjątek jeśli wystąpi błąd
 
  $db = new DbManager();
  $data = $db->fetch(...);
  //rób coś z $data
  $data = $db->fetchAll(...);
  //rób coś z $data
} catch (Exception $e){
  //obsługa błędów. Tutaj wskoczymy jeśli którakolwiek z metod wyrzuci wyjątek
}
[PHP] pobierz, plaintext 

Tylko po co te nakładki? Tylko po to, aby kod był "krótki" ? I dane połączenia tworzą się same w konstruktorze?

@-=Peter=-
Dzieki za wskazówki i pięknie przedstawiony kod.

Ale wychodzi na to ze now musze klepac try {} catch {} na kazdym kroku...
A moze polecice jakis artykól o wyjątkach i ich traktowaniu ?

http://pl.wikibooks.org/wiki/PHP/Wyjątki

To naleciałość z PHP4, aby oznaczyć daną właściwość jako prywatną, teraz ze względu na modyfikatory dostępu to nie ma sensu.