Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Poprawność konstruktora?
Forum PHP.pl > Forum > PHP > Object-oriented programming
Nortonek
Witam

W tej chwili konstruktor wymusza 3 połączenie do bazy, czy taki zapis konstruktora jest poprawny, czy można to zoptymalizować?

  1. <?php
  2.  
  3. $id = 1;
  4.  
  5. $Player = new Player($id);
  6.  
  7. $USER['nick'] = $Player->nick;
  8. $USER['level'] = $Player->level;
  9. $USER['praca'] = $Player->praca;
  10. $USER['kasa'] = $Player->kasa;
  11.  
  12. echo $USER['kasa'];
  13.  
  14.  
  15.  
  16.  
  17. class Players {
  18. public $nick;
  19. public $level;
  20. public $id_players;
  21. public $kasa;
  22. public $praca;
  23.  
  24.  
  25. function __construct($id) {
  26. $this -> id_players= $id;
  27. self::get_dane();
  28. self::update_kasa();
  29. self::get_dane();
  30. }
  31.  
  32. public function get_dane(){
  33. global $db, $table_players;
  34.  
  35. $stats = $db -> Execute("SELECT * FROM $table_players WHERE `id_players`='".$this -> id_players."' AND `aktywny`='1' ");
  36. $this -> nick = $stats -> fields['nick'];
  37. $this -> level= $stats -> fields['level'];
  38. $this -> kasa= $stats -> fields['kasa'];
  39. $this -> praca= $stats -> fields['praca'];
  40. $stats -> Close();
  41. }
  42.  
  43. public function update_kasa(){
  44. global $db, $table_players;
  45.  
  46. if ( $this -> praca <= time() ) {
  47. $kasa = 100;
  48. $result = $db -> Execute("UPDATE $table_players SET `kasa`= `kasa` + '$kasa', `praca`='0' WHERE `id_players`='".$this -> id_players."' ");
  49. $result -> Close();
  50. }
  51. }
  52.  
  53.  
  54. }
  55.  
  56. ?>
Turson
Napisałeś klasę, która wymusza jakaś akcję już w konstruktorze. To nie najlepszy pomysł, bo za to powinny odpowiadać poszczególne metody. Powiedzmy, że będziesz chciał pobrać same dane użytkownika metodą get_dane(), ale klasa przy okazji zrobi update w bazie. Kiepsko. Metoda get_dane() powinna przyjmować jakiś parametr np. id usera w przypadku, gdy będziesz chciał wykonać jedynie pobranie danych.
Nortonek
Wymyśliłem sobie że update ma wykonywać się przy każdym odświeżeniu strony,

podczas gry często się zmienia stan zmiennych dla Playera

Po wykonaniu update pokazuje stan zmiennych ...

Dodałem do klasy warunek, który zmniejsza o 1 zapytanie do bazy, zaczynam od zera OOP to pierwsze kroki i dlatego tak się dopytuję czy dobrze kombinuję

  1. <?php
  2.  
  3. $id = 1;
  4.  
  5. $Player = new player($id);
  6.  
  7. $USER['nick'] = $Player->nick;
  8. $USER['level'] = $Player->level;
  9. $USER['praca'] = $Player->praca;
  10. $USER['kasa'] = $Player->kasa;
  11.  
  12. echo $USER['kasa'];
  13.  
  14.  
  15.  
  16.  
  17. class Players {
  18. public $nick;
  19. public $level;
  20. public $id_players;
  21. public $kasa;
  22. public $praca;
  23. public $byl_update; /* DODANO */
  24.  
  25.  
  26. function __construct($id) {
  27. $this->byl_update = 1; /* DODANO */
  28. $this -> id_players= $id;
  29. self::get_dane();
  30. self::update_kasa();
  31. if ($this->byl_update==2) self::get_dane(); /* DODANO */
  32. }
  33.  
  34. public function get_dane(){
  35. global $db, $table_players;
  36.  
  37. $stats = $db -> Execute("SELECT * FROM $table_players WHERE `id_players`='".$this -> id_players."' AND `aktywny`='1' ");
  38. $this -> nick = $stats -> fields['nick'];
  39. $this -> level= $stats -> fields['level'];
  40. $this -> kasa= $stats -> fields['kasa'];
  41. $this -> praca= $stats -> fields['praca'];
  42. $stats -> Close();
  43. }
  44.  
  45. public function update_kasa(){
  46. global $db, $table_players;
  47.  
  48. if ( $this -> praca <= time() ) {
  49. $kasa = 100;
  50. $result = $db -> Execute("UPDATE $table_players SET `kasa`= `kasa` + '$kasa', `praca`='0' WHERE `id_players`='".$this -> id_players."' ");
  51. $result -> Close();
  52. $this->byl_update = 2; /* DODANO */
  53. }
  54. }
  55.  
  56.  
  57. }
  58.  
  59. ?>


pyro
W przytoczonym kodzie myślę, że tak na oko znajduje się przynajmniej z 10-15 nieprawidłowości. Nie chce mi się ich wszystkich wymieniać, ale dam pewną radę - poszukaj gotowej (i sensownej) klasy w internecie, porównaj, wyciągnij wnioski.
Nortonek
trudno jest szukać jeśli nie ma się pojęcia czego sadsmiley02.gif może jakaś wskazówka co do błędów

tą klasę uważałem za cud techniki
Turson
Już dałem Ci odpowiedź, że klasa jest bez sensu i kontruktor robi niepotrzebne zadania. Za update, czy pobranie danych odpowiadają metody. W przytoczonym przez Ciebie przykładzie stają się one bezużyteczne biorąc pod uwagę ich jednostkowe wykorzystanie w innym kodzie.
Jak mówiłem, nie pobierzesz samych danych, nie zrobisz samego update bo to suche metody bez żadnych parametrów.
Konstruktor możesz wywalić, a zrobić jedynie same metody do pobrania/update, które przyjmują jakiś parametr (najlepiej id usera). Wtedy robisz instancję obiektu i operujesz na nim $obiekt->getDane(u$ser_id). Oczywiście ta metoda musiałaby coś zwracać.
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.