Klasa do oceny - początkujący Opcje

[GreenGo]

Witam,
napisałem sobie taką małą klasę, która odpowiada za pozycję użytkownika na mapie i chciałbym spytać czy to w ogóle ma coś wspólnego z OOP, co poprawić, jak ją lepiej napisać?

[PHP] pobierz, plaintext 
 
class Map {
	private $position_x;
	private $position_y;
	private $uesr_id;
	private $action_query;
 
	public function __construct()
	{
		$this->position_x 	= $_POST['position_x'];
		$this->position_y 	= $_POST['position_y'];
		$this->user_id    	= $_SESSION['id'];
	}
 
	public function doDate ()
       // funkcja odpowiada za rozdzielenie daty z postaci unixowej na godziny minuty i sekundy abym mógł potem przekazać je do odliczania w JS.
      // w tabeli 'aciotn' przechowuję id użytkonika poruszającego się po mapie, nowe współrzędne oraz czas rozpoczęcia i zakończenia wędrówki	
      {
		$record_exist = mysql_query("SELECT * FROM `action` WHERE userid = '{$this->user_id}'");
		if(mysql_affected_rows() > 0){
 
			$w = mysql_fetch_array($record_exist);
				$last_sec = $w['end_time'] - time();
 
				$last_day  = floor($last_sec/86400);
				$last_hour = floor(($last_sec - $last_day*86400)/3600);
				$last_min  = floor(($last_sec - $last_day*86400 - $last_hour*3600)/60);
				$last_sec  = $last_sec - $last_day*86400 - $last_hour*3600 - $last_min*60;
 
				$time = array($last_hour, $last_min, $last_sec);
				return $time;
		}
		else
			return false;
	}
 
 
        public function endTime ()
	//funkcja odpowiada za obliczenie czasu zakończenia wędrówki na podstawie współrzędnych przed rozpoczęciem wędrówki jak i miejsca
       //docelowego
        {
		$end_time_query = mysql_query("SELECT * FROM `character` WHERE userid = '$this->user_id'");
		$w = mysql_fetch_array($end_time_query);
		$position  = $w['position'];
		$pos = explode(",", $position);
 
		$end_time = time() + round(sqrt(pow(abs($pos[0] - $this->position_x), 2) + pow(abs($pos[1] - $this->position_y), 2)), 2);
		return $end_time; 
	}
 
	public function position ()
	// w bazie przechowuje współrzędne w postaci  np:  450,655. Ta funkcja wyświetla pozycje wszystkich graczy rozdzielając współrzędne na x i y.
 
       {
 
	$position_query = mysql_query("SELECT * FROM `character` ");
	$i = 0;
	$arr = array();
	while ($w = mysql_fetch_array($position_query)) {
      	$position  = $w['position'];
		$pos = explode(",", $position);
 
		$arr[$i] = $pos[0];
		$arr[$i+1] = $pos[1];
		$i+=2;
	}
	return $arr ;
	}
 
	public function changePosition()
	// dodanie do tabeli action informacji o danym poruszaniu się (kto, o której, do której, i gdzie)
       {
		$start_time = time();
		mysql_query("INSERT INTO action (userid, start_time, end_time, new_position) value ('{$this->user_id}', '$start_time', '{$this->endTime()}', '{$this->position_x},{$this->position_y}')");
	}
 
	public function updatePosition()
       // po odliczeniu czasu w jakim uzytkownik będzie się przemieszczał na nową lokalizację funkca ta usunie akcje z tabeli "action" i uaktualni obecne położenie uzytkownika	
{
		$update_position_query = mysql_query("SELECT * FROM `action` WHERE userid = '{$this->user_id}'");
		$w = mysql_fetch_array($update_position_query);
		if(mysql_affected_rows() > 0 AND time() >= $w['end_time']){
 
 
		$new_position = $w['new_position'];
		mysql_query("UPDATE `character` SET `position` = '$new_position' WHERE userid = '{$this->user_id}'");
		mysql_query("DELETE FROM `action` WHERE `userid` = '{$this->user_id}'");
 
		}
 
	}
}
[PHP] pobierz, plaintext 

Dopiero zaczyna pisać klasy w php i chciałbym nabierać dobrych nawyków więc byłbym wdzięczny za uwagi (IMG:style_emoticons/default/smile.gif)
Pozdrawiam.

[zegarek84]

ważne, że zacząłeś (IMG:style_emoticons/default/winksmiley.jpg) - gdy będziesz w kodzie miał po kilka różnych klas wtedy lepiej to wypracujesz... gdy zobaczysz podobieństwa w końcu zaczniesz korzystać z dziedziczenia... przede wszystkim trzeba pisać...
ale najważniejsza uwaga to zamiast:

[PHP] pobierz, plaintext 
	public function __construct()
	{
		$this->position_x 	= $_POST['position_x'];
		$this->position_y 	= $_POST['position_y'];
		$this->user_id    	= $_SESSION['id'];
	}
[PHP] pobierz, plaintext 

powinno być:

[PHP] pobierz, plaintext 
	public function __construct($x, $y, $id)
	{
		$this->position_x 	= $x;
		$this->position_y 	= $y;
		$this->user_id    	= $id;
	}
[PHP] pobierz, plaintext 

no i argumenty podajesz przy konstrukcji klasy... niekture jakoś mogą być wcześniej ustawione jeśli miałbyś podawać czasem mniejszą liczbę argumentów np. __construct($x, $y, $id = 0)... no i w klasie możesz sprawdzać, czy zostały zadeklarowane odpowiednie zmienne - jeśli nie to rzucić jakimiś wyjątkami i takie tam...

a i przy metodach, gdzie nic nie zwracasz konkretnego a jest możliwe, ze coś jeszcze z klasa będziesz robił można by dać return $this; ... co pozwoli czasem na konstrukcję $klasa->ustaw_cos()->zmien_imie('staszek')->ile_lat()..... [tzn przy zmianie imienia można było zwrucić np stare imię lub czy się powiodło - jeśli się powiodło to this a jeśli nie to null bądź false i inne tam - taki sobie łańcuszek na szybko napisałem ;p]

Ten post edytował zegarek84 25.05.2010, 16:07:26

[zend]

Wiesz do czego służy funkcja mysql_affected_rows()? Bynajmniej nie do sprawdzania ile danych zostało pobranych w select'ie.

[PHP] pobierz, plaintext 
class Map
{	
	public function getPositions()
	{
		return array(new Position(1,2 , 10) , new Position(3,4 , 15));
	}
 
	public function getUserPosition($id)
	{
		return Position(1,2,3);
	}
}
 
class Position
{
 
	public function __construct($x = null , $y = null , $id) {}
 
	public function move($x , $y) {}
 
	public function getX() {}
 
	public function getY() {}
}
[PHP] pobierz, plaintext 

Spróbuj zaimplementować wnętrza metod które podałem i sam oceń czy mój czy twój kod jest bardziej elastyczny (IMG:style_emoticons/default/smile.gif)

Edit:
poprawienie metod

W konstuktorze x i y są dodatkowo null'ami żebyś mógł stworzyć nowy obiekt nie znając wartości na początku a potem mogł przestawić postać

[PHP] pobierz, plaintext 
$p = new Position(null , null , 20);
$p -> move(100 , 200);
[PHP] pobierz, plaintext 

Ten post edytował zend 26.05.2010, 11:46:19

[zegarek84]

kolega wyżej dał kolejny dobry myk [bardziej mam na myśli definiowanie metod i funkcji - może być tak jak tutaj argumentów konstruktora], ale zamiast:

[PHP] pobierz, plaintext 
class Position
{
	public function __construct($x = null , $y = null , $id) {}
//....
}
[PHP] pobierz, plaintext 

W konstuktorze x i y są dodatkowo null'ami żebyś mógł stworzyć nowy obiekt nie znając wartości na początku a potem mogł przestawić postać

[PHP] pobierz, plaintext 
$p = new Position(null , null , 20);
$p -> move(100 , 200);
[PHP] pobierz, plaintext 

powinno się pisać:

[PHP] pobierz, plaintext 
class Position
{
	public function __construct($id, $x = null , $y = null) {}
//....
}
[PHP] pobierz, plaintext 

i nie mam na myśli przy tej klasie konkretnie... a dlaczego tak?? by móc potem pisać:

[PHP] pobierz, plaintext 
$p = new Position($tylko_jakies_id); //jeden argument
$p2 = new Position($jakies_id, $jakis_x); // 2 argumenty
$p3 = new Position($jakies_id, $jakis_x, $jakis_y); // 3 argumenty
// a w rzeczywistości konstruktor otrzymuje zawsze 3 argumenty, jednak jeśli czegoś nie podajesz dostawał domyślny null dla pozostałych
[PHP] pobierz, plaintext 

oczywiście równie dobrze można to zdefiniować jako:

public function __construct($id, $x = domyslna_wartosc , $y = domyslna_wartosc) {}

no i jeszcze jeśli byś chciał przez argument przekazywać tylko określoną klasę bądź jej potomki bądź mającą określony interfejs to można jeszcze coś takiego:

Kod

funkcja_lub_metoda_czy_konstruktor (Nazwa_Klasy_Interfejsu_badz_Rodzica $zmienna_reprezentujaca_obiekt /* można jej dać jeszcze domyślną wartość np jako null czyli = null */) {...}

Ten post edytował zegarek84 27.05.2010, 14:06:42

[gcdreak]

1. Pisz settery i gettery jeśli są konieczne.
2. Pisz komentarze w stylu PhpDoc:

[PHP] pobierz, plaintext 
               /**
		 * Set query type as SELECT and set fields using in query.
		 * 
		 * @see setFields()
		 * @param mixed	$fields	Array or string with fields using in query.
		 * @return object Generator
		 */
		public function select($fields) {
			$this->setFields($fields);
 
			$this->sqlAction = 'SELECT';
 
			return $this;
		}
[PHP] pobierz, plaintext 

3. Korzystaj z dodatkowej warstwy abstracji do komunikacji z bazą danych.
4. Jeśli w if...else używasz klamer to używaj ich i do else i do if:

[PHP] pobierz, plaintext 
//  poprawnie wg mnie
if( $cos === true) {
    //....
} else {
   //...
}
if( $cos === true) 
    //....
else
   //...
 
// niepoprawne wg mnie
if( $cos === true) {
    //....
} else 
   //...
 
 
[PHP] pobierz, plaintext 

[Cysiaczek]

Popatrz krytycznie na kod odpytujący bazę danych.
1. Zapytania powinny być wykonywane przez jakiś osobny obiekt, np. http://www.php.net/manual/en/book.pdo.php
2. Brak obsługi błędów (zapytania się czasami wywalają)