Forum PHP.pl

Witam.
Mam na imię Tomek, lat mam 15 skończone 11 grudnia.
PHP zajmuje się parę miesięcy a OOP może miesiąc, akurat tym językiem, ponieważ uważam że on jest jak dla mnie najłtawiejszy

Napisałem klasę MySQL w celu edukacyjnym, Proszę o ocene.
Krytyka jak najmilej widziana ( Tylko nie do przesady ).

[PHP] pobierz, plaintext 
<?php error_reporting( E_ALL ^ E_NOTICE );
// +---------------------------------------------------------------------------+
// | Copyright (c) 2012 Starmi                                       |
// +---------------------------------------------------------------------------+
class Db
{
	/**
	 * @var array tablica zawierające dane typu host/user/base/pass itp...
	 */
	var $_params = array();
 
	/**
	 * @var bool Czy klasa jest połączona z bazą danych
	 */
	var $_connect = false;
 
	/**
	 * @var link Uchwyt połaczenia bazy danych
	 */
	var $_connect_link = null;
 
	/**
	 * @var resource_id Uchwyt ostatniego zapytania mysql
	 */
	var $_last_query = null;
 
	/**
	 * Zmienia dane w zmiennej globalnej $_params
	 *
	 * @param array|string $name klucz lub tablica
	 * @param string $value jeśli $name nie jest tablicą to używany tej zmiennej do przypisania wartości
	 * @return bool zwraca czy dane zostały poprawnie zmienione/dodane do $_params
	 * @acces public
	 */
	function setParam( $name, $value = null )
	{
		if( is_null($name) )
		{
			return false;
		}
 
		if( is_array($name) )
		{
			foreach($name as $key => $value)
			{
				if( empty($key) || empty($value) )
				{
					continue;
				}
 
				$this -> _params[ $key ] = $value;
 
			}
			return true;
		}
 
		$this -> _params[ $name ] = $value; return true;
	}
 
	/**
	 * Funkcja pobiera dane z tablicy globalnej $_params
	 *
	 * @param array|string klucz lub tablica zawierająca klucze...
	 * @return array|string zwraca wyniki
	 * @acces public
	 */
	function getParam( $param = null )
	{
		if( is_array($param) )
		{
			$_return = array();
			foreach($param as $param_key)
			{
				if( isset($this->_params[ $param_key ]) && !is_null($param_key) )
				{
					$_return[ $param_key ] = $this -> _params[$param_key];
				}
				else continue;
			}
			return $_return;
		}
 
		return $this -> _params[$param];
	}
 
	/**
	 * Funkcja odpowiadająca za łączenie z bazą danych
	 *
	 * @return bool Zwraca wynik połączenia...
	 * @acces public
	 */
	function connect()
	{
		$db = $this -> getParam(array('host', 'user', 'pass', 'base'));
 
		if( ($connect_id = @mysql_connect($db['host'], $db['user'], $db['pass']) ) )
		{
			if( @mysql_select_db($db['base'], $connect_id) )
			{
				$this -> _connect = true;
				$this -> _connect_link = $connect_id;
				return true;
			}
			else
			{
				$this -> writeError('Nie można wybrać bazy danych...');
			}
		}
		else
		{
			$this -> writeError('Nie można połączyć się z bazą danych');
		}
	}
 
	/**
	 * Wypisuje błąd na stronie
	 *
	 * @param string $error błąd do wyświetlenia
	 * @return bool wysyła wartość zwróconą przez trigger_error()
	 */
	function writeError( $error ){
		return trigger_error($error, E_USER_ERROR);
	}
 
	/**
	 * Wysyła zapytanie do MySQL
	 *
	 * @param string $query_string string jaki zostanie wysłany do mysql
	 * @param bool   $fetch czy zwrócić tablice z wynikami ['fetch']
	 * @param bool   $rows czy zwrócić ilość rekordów w bazie danych ['rows']
	 * @return bool|array false jeśli zapytanie się nie powiedzie, array jeśli tak
	 * @acces publoc
	 */
	function query( $query_string = null, $fetch = FALSE, $rows = FALSE )
	{
		if( is_null($query_string) )
		{
			return false;
		}
 
		unset( $this -> _last_query );
		if( $query = @mysql_query($query_string, $this->_connect_link) )
		{
			$this -> _last_query = $query;
			switch( TRUE )
			{
				case $fetch:
					$_return['fetch'] = mysql_fetch_array( $this -> _last_query );
				break;
				case $rows:
					$_return['rows'] = mysql_num_rows( $this -> _last_query );
				break;
			}
			$_return['handle'] = $this -> _last_query;
			return $_return;
		}
		else
		{
				$this -> writeError( 'Nie można wykonać zapytania mysql </br>Zapytanie: ' . $query_string );
		}
	}
 
	/**
	 * Dodaje rekord do bazy danych
	 *
	 * Możesz użyć funkcji query, ale po co? ta jest łatwiejsza !  Nie zwraca żadnej wartości
	 * 
	 * @param string $table tabela do jakies ma zostać dodany rekord
	 * @param array $insert_data dane jakie zostaną dodane np: array('kolumna' => 'wartość')
	 */
	function insert( $table, $insert_data )
	{
		$this -> query( sprintf("INSERT INTO `%s` (`%s`) VALUES(\"%s\")",
			$table,
			implode('`, `', array_keys($insert_data)),
			implode('", "', addslashes(array_values($insert_data))
		)));
	}
 
 
}
 
/** Przykład użycia... **/
$db = new Db();
$db -> setParam('host', 'localhost');
$db -> setParam('pass', '');
$db -> setParam('user', 'root');
$db -> setParam('base', 'test');
$db -> connect();
 
[PHP] pobierz, plaintext 

Temat ten sam napisałem w dziale php lecz niestety on został zamknięty Ponieważ moderator nie zrozumiał posta, a ja chciałem tylko ocenę

Bez sensu zupełnie.

- Skoro jest to klasa do obsługi jednego połączenia, to po co całe te setParam(). Te wszystkie dane powinno się przekazać w konstruktorze. Nawet w przypadku, jakby była to klasa do obsługi wielu połączeń to powinna być jakaś tablica z połączeniami

[PHP] pobierz, plaintext 
$db = new Db();
$db->newConnection('host', 'user', 'password', 'database'); // W klasie: $this->connections[] = ...
[PHP] pobierz, plaintext 

- Po co ta metoda getParam() ? Jakby programista zapomniał jakie dane wpisał do połączenia z bazą danych w międzyczasie czy jak?
- Mało mówiące nazwy metod / pól. ($_connection, co connection? Mogłoby być coś w stylu $is_connected, setParam(), getParam() parametry czego?). Niby jest opis z PHPDoc, ale przy większych skryptach / projektach potem ciężko się połapać.
- metoda query(), a potem oddzielna dla samego INSERT? Musisz się zdecydować. Albo zostawiasz funkcję query() (tyle, że jakoś lepiej skonstruowaną), albo dla zapytań robisz oddzielne select(), insert(), update() itd.
- Brak jakichkolwiek public / protected / private. PHP4 to już przeszłość.

// ADD

Dodatkowo używanie addslashes(), zamiast (real_)escape_string();

Ten post edytował pyro 22.12.2012, 14:46:40