Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Klasa do oceny - początkujący
Forum PHP.pl > Forum > PHP > Object-oriented programming
GreenGo
25.05.2010, 15:28:01
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 
  1.  
  2. class Map {
  3. 	private $position_x;
  4. 	private $position_y;
  5. 	private $uesr_id;
  6. 	private $action_query;
  7.  
  8. 	public function __construct()
  9. 	{
  10. 		$this->position_x 	= $_POST['position_x'];
  11. 		$this->position_y 	= $_POST['position_y'];
  12. 		$this->user_id    	= $_SESSION['id'];
  13. 	}
  14.  
  15. 	public function doDate ()
  16.        // funkcja odpowiada za rozdzielenie daty z postaci unixowej na godziny minuty i sekundy abym mógł potem przekazać je do odliczania w JS.
  17.       // 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	
  18.       {
  19. 		$record_exist = mysql_query("SELECT * FROM `action` WHERE userid = '{$this->user_id}'");
  20. 		if(mysql_affected_rows() > 0){
  21.  
  22. 			$w = mysql_fetch_array($record_exist);
  23. 				$last_sec = $w['end_time'] - time();
  24.  
  25. 				$last_day  = floor($last_sec/86400);
  26. 				$last_hour = floor(($last_sec - $last_day*86400)/3600);
  27. 				$last_min  = floor(($last_sec - $last_day*86400 - $last_hour*3600)/60);
  28. 				$last_sec  = $last_sec - $last_day*86400 - $last_hour*3600 - $last_min*60;
  29.  
  30. 				$time = array($last_hour, $last_min, $last_sec);
  31. 				return $time;
  32. 		}
  33. 		else
  34. 			return false;
  35. 	}
  36.  
  37.  
  38.         public function endTime ()
  39. 	//funkcja odpowiada za obliczenie czasu zakończenia wędrówki na podstawie współrzędnych przed rozpoczęciem wędrówki jak i miejsca
  40.        //docelowego
  41.         {
  42. 		$end_time_query = mysql_query("SELECT * FROM `character` WHERE userid = '$this->user_id'");
  43. 		$w = mysql_fetch_array($end_time_query);
  44. 		$position  = $w['position'];
  45. 		$pos = explode(",", $position);
  46.  
  47. 		$end_time = time() + round(sqrt(pow(abs($pos[0] - $this->position_x), 2) + pow(abs($pos[1] - $this->position_y), 2)), 2);
  48. 		return $end_time; 
  49. 	}
  50.  
  51. 	public function position ()
  52. 	// 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.
  53.  
  54.        {
  55.  
  56. 	$position_query = mysql_query("SELECT * FROM `character` ");
  57. 	$i = 0;
  58. 	$arr = array();
  59. 	while ($w = mysql_fetch_array($position_query)) {
  60.       	$position  = $w['position'];
  61. 		$pos = explode(",", $position);
  62.  
  63. 		$arr[$i] = $pos[0];
  64. 		$arr[$i+1] = $pos[1];
  65. 		$i+=2;
  66. 	}
  67. 	return $arr ;
  68. 	}
  69.  
  70. 	public function changePosition()
  71. 	// dodanie do tabeli action informacji o danym poruszaniu się (kto, o której, do której, i gdzie)
  72.        {
  73. 		$start_time = time();
  74. 		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}')");
  75. 	}
  76.  
  77. 	public function updatePosition()
  78.        // 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	
  79. {
  80. 		$update_position_query = mysql_query("SELECT * FROM `action` WHERE userid = '{$this->user_id}'");
  81. 		$w = mysql_fetch_array($update_position_query);
  82. 		if(mysql_affected_rows() > 0 AND time() >= $w['end_time']){
  83.  
  84.  
  85. 		$new_position = $w['new_position'];
  86. 		mysql_query("UPDATE `character` SET `position` = '$new_position' WHERE userid = '{$this->user_id}'");
  87. 		mysql_query("DELETE FROM `action` WHERE `userid` = '{$this->user_id}'");
  88.  
  89. 		}
  90.  
  91. 	}
  92. }
[PHP] pobierz, plaintext


Dopiero zaczyna pisać klasy w php i chciałbym nabierać dobrych nawyków więc byłbym wdzięczny za uwagi smile.gif
Pozdrawiam.
zegarek84
25.05.2010, 16:03:12
ważne, że zacząłeś 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 
  1. 	public function __construct()
  2. 	{
  3. 		$this->position_x 	= $_POST['position_x'];
  4. 		$this->position_y 	= $_POST['position_y'];
  5. 		$this->user_id    	= $_SESSION['id'];
  6. 	}
[PHP] pobierz, plaintext

powinno być:
[PHP] pobierz, plaintext 
  1. 	public function __construct($x, $y, $id)
  2. 	{
  3. 		$this->position_x 	= $x;
  4. 		$this->position_y 	= $y;
  5. 		$this->user_id    	= $id;
  6. 	}
[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]
zend
25.05.2010, 22:36:04
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 
  1. class Map
  2. {	
  3. 	public function getPositions()
  4. 	{
  5. 		return array(new Position(1,2 , 10) , new Position(3,4 , 15));
  6. 	}
  7.  
  8. 	public function getUserPosition($id)
  9. 	{
  10. 		return Position(1,2,3);
  11. 	}
  12. }
  13.  
  14. class Position
  15. {
  16.  
  17. 	public function __construct($x = null , $y = null , $id) {}
  18.  
  19. 	public function move($x , $y) {}
  20.  
  21. 	public function getX() {}
  22.  
  23. 	public function getY() {}
  24. }
[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 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 
  1. $p = new Position(null , null , 20);
  2. $p -> move(100 , 200);
[PHP] pobierz, plaintext
zegarek84
27.05.2010, 14:05:35
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:
Cytat(zend @ 25.05.2010, 23:36:04 ) *
[PHP] pobierz, plaintext 
  1. class Position
  2. {
  3. 	public function __construct($x = null , $y = null , $id) {}
  4. //....
  5. }
[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 
  1. $p = new Position(null , null , 20);
  2. $p -> move(100 , 200);
[PHP] pobierz, plaintext

powinno się pisać:
[PHP] pobierz, plaintext 
  1. class Position
  2. {
  3. 	public function __construct($id, $x = null , $y = null) {}
  4. //....
  5. }
[PHP] pobierz, plaintext
i nie mam na myśli przy tej klasie konkretnie... a dlaczego tak?? by móc potem pisać:
[PHP] pobierz, plaintext 
  1. $p = new Position($tylko_jakies_id); //jeden argument
  2. $p2 = new Position($jakies_id, $jakis_x); // 2 argumenty
  3. $p3 = new Position($jakies_id, $jakis_x, $jakis_y); // 3 argumenty
  4. // 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 */) {...}
gcdreak
8.07.2010, 22:25:26
1. Pisz settery i gettery jeśli są konieczne.
2. Pisz komentarze w stylu PhpDoc:
[PHP] pobierz, plaintext 
  1.                /**
  2. 		 * Set query type as SELECT and set fields using in query.
  3. 		 * 
  4. 		 * @see setFields()
  5. 		 * @param mixed	$fields	Array or string with fields using in query.
  6. 		 * @return object Generator
  7. 		 */
  8. 		public function select($fields) {
  9. 			$this->setFields($fields);
  10.  
  11. 			$this->sqlAction = 'SELECT';
  12.  
  13. 			return $this;
  14. 		}
[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 
  1. //  poprawnie wg mnie
  2. if( $cos === true) {
  3.     //....
  4. } else {
  5.    //...
  6. }
  7. if( $cos === true) 
  8.     //....
  9. else
  10.    //...
  11.  
  12. // niepoprawne wg mnie
  13. if( $cos === true) {
  14.     //....
  15. } else 
  16.    //...
  17.  
  18.  
[PHP] pobierz, plaintext
Cysiaczek
11.07.2010, 03:48:15
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ą)
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.