Poprawność konstruktora? Opcje

[Nortonek]

Witam

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

[PHP] pobierz, plaintext 
<?php
 
  $id = 1;
 
  $Player = new Player($id);
 
  $USER['nick']   = $Player->nick;
  $USER['level']  = $Player->level;
  $USER['praca'] = $Player->praca;
  $USER['kasa']  = $Player->kasa;
 
  echo $USER['kasa'];
 
 
 
 
class Players {
  public $nick;
  public $level;
  public $id_players;
  public $kasa;
  public $praca;
 
 
  function __construct($id)   {  
      $this -> id_players= $id;
      self::get_dane();
      self::update_kasa();
      self::get_dane();
  }
 
  public function get_dane(){
    global $db, $table_players;
 
        $stats = $db -> Execute("SELECT *  FROM  $table_players 	WHERE 	`id_players`='".$this -> id_players."'  AND `aktywny`='1'	");
        $this -> nick = $stats -> fields['nick'];
        $this -> level= $stats -> fields['level'];
        $this -> kasa= $stats -> fields['kasa'];
				$this -> praca= $stats -> fields['praca'];
        $stats -> Close();
  }
 
  public function update_kasa(){
		global $db, $table_players;
 
    if ( $this -> praca <= time() ) {
        $kasa = 100;
        $result = $db -> Execute("UPDATE $table_players SET `kasa`= `kasa` + '$kasa', `praca`='0' WHERE `id_players`='".$this -> id_players."' ");
        $result -> Close();
    }
  }
 
 
}
 
?>
[PHP] pobierz, plaintext 

[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ę

[PHP] pobierz, plaintext 
<?php
 
  $id = 1;
 
  $Player = new player($id);
 
  $USER['nick']   = $Player->nick;
  $USER['level']  = $Player->level;
  $USER['praca'] = $Player->praca;
  $USER['kasa']  = $Player->kasa;
 
  echo $USER['kasa'];
 
 
 
 
class Players {
  public $nick;
  public $level;
  public $id_players;
  public $kasa;
  public $praca;
	public $byl_update;                                             /* DODANO */
 
 
  function __construct($id)   {  
	    $this->byl_update = 1;                                             /* DODANO */
      $this -> id_players= $id;
      self::get_dane();
      self::update_kasa();
      if ($this->byl_update==2) self::get_dane();                                             /* DODANO */
  }
 
  public function get_dane(){
    global $db, $table_players;
 
        $stats = $db -> Execute("SELECT *  FROM  $table_players 	WHERE 	`id_players`='".$this -> id_players."'  AND `aktywny`='1'	");
        $this -> nick = $stats -> fields['nick'];
        $this -> level= $stats -> fields['level'];
        $this -> kasa= $stats -> fields['kasa'];
				$this -> praca= $stats -> fields['praca'];
        $stats -> Close();
  }
 
  public function update_kasa(){
		global $db, $table_players;
 
    if ( $this -> praca <= time() ) {
        $kasa = 100;
        $result = $db -> Execute("UPDATE $table_players SET `kasa`= `kasa` + '$kasa', `praca`='0' WHERE `id_players`='".$this -> id_players."' ");
        $result -> Close();
				$this->byl_update = 2;                                             /* DODANO */
    }
  }
 
 
}
 
?>
[PHP] pobierz, plaintext 

Ten post edytował Nortonek 12.02.2014, 21:18:45

[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 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ć.