Bitte um Stellungnahme zu meiner ersten Klasse

Nord-Süd-Richtung

Erfahrenes Mitglied
Hi,
wie man dem Titel entnehmen kann habe ich heute mal versucht meine erste Klasse auf die Beine zu stellen. Ich wollte euch mal darum bitten, Stellung dazu zu nehmen, sowie Verbesserungsvorschläge zu äußern.
PHP:
<?php
class mysql{
  private connect_id;
  private $db;
  const host = 'localhost';
  const user = '*********';
  const pass = '*********';
  const db = '*********';
  function mysql(){
  }
  private function connect(){
    $this->connect_id = mysql_connect(host,user,pass);
    $this->db = mysql_select_db(db);
    return $connect_id;
  }
}
class select extends mysql{
  private $ret_arr;
  const RET_ARRAY = 'RET_ARRAY';
  const RET_ASSOC = 'RET_ASSOC';
  const RET_OBJECT = 'RET_OBJECT';
  public function select($query, ret_type){
    $connect_id = $this->connect();
    $getData = mysql_query($query,$connect_id);
    if( $getData == false )
      return false;
    switch( ret_type ){
      case RET_ARRAY:
        while( $data = mysql_fetch_array($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
      case RET_ASSOC:
        while( $data = mysql_fetch_assoc($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
      case RET_OBJECT:
        while( $data = mysql_fetch_object($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
    }
  }
}
 
Es ist meiner Meinung nach wenig sinnvoll eine Klasse zu erstellen die lediglich die Verbindung aufbaut und dann eine zweite Klasse von dieser erben zu lassen und dort dann die Abfrage zu implementieren.
Das kann man auch gleich in einer Klasse machen, denn beides macht allein eh wenig Sinn.

Zudem sehe ich folgende Probleme:
  • Um die Verbindungsdaten anzugeben muss der Klassen-Code editiert werden. Das ist ungemein unpraktisch, denn eine Klasse sollte eher eine Art Blackbox sein in die der Entwickler nicht reinschauen muss um damit arbeiten zu koennen.
    Die Uebergabe der Verbindungsdaten ueber den Constructor ist weitaus sinnvoller und flexibler.
  • Die in der Klasse select deklarierten Konstanten sind, wenn ich mich nicht irre, nicht global verfuegbar und somit wird der Entwickler Probleme haben den richtigen Parameter an die Methode select(). Zudem waere hier auch sinnvoll den 2. Parameter optional zu machen indem Du einen Default-Wert angibst.

In der aktuellen Version finde ich die Klasse(n) wenig sinnvoll. Als Beispiel fuer eine Datenbank-Klasse verweise ich an dieser Stelle einfach mal auf meine Klasse MultiSQL.
 
Hi

Vielen Dank für deine Antwort, ich habe meine Klasse nun einmal umgeschrieben, und versucht deine angesprochenen Mängel zu beheben.
PHP:
<?php
class mysql{
  private connect_id;
  private $db;
  private $ret_arr;
  private $host;
  private $user;
  private $pass;
  private $db;
  function mysql($host,$user,$pass,$db){
    $this->host = $host;
    $this->user = $user;
    $this->pass = $pass;
    $this->db = $db;
  }
  public function connect(){
    $this->connect_id = mysql_connect($host,$user,$pass);
    $this->db = mysql_select_db($db);
    return $connect_id;
  }
  public function select($query, $ret_type = 2){
    $getData = mysql_query($query,$this->$connect_id);
    if( $getData == false )
      return false;
    switch( $ret_type ){
      case 1:
        while( $data = mysql_fetch_array($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
      case 2:
        while( $data = mysql_fetch_assoc($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
      case 3:
        while( $data = mysql_fetch_object($getData) )
          $ret_arr[] = $data;
        return $ret_arr;
    }
  }
}
 
Warum der MIschMasch aus PHP4 und PHP5?

[phpf]__construct[/phpf] mal als Beispiel sollte genutzt werden.

Wenn du Sichtbarkeitsoperatoren nutzt, dann solltest du sie konsequent nutzen und nicht nur manchmal.
 
Hi
Der Mischmasch aus PHP4 und 5 kommt warscheinlich daher, dass ich mir mein kleines Wissen von einer PHP4 und von einer PHP5 Seite angeeignet habe, im Nachhinein keine kluge Idee... Nichtsdestotrotz habe ich versucht deinen Ratschlag umzusetzen:
PHP:
<?php
class mysql{
  private connect_id;
  private $db;
  private $ret_arr;
  private $host;
  private $user;
  private $pass;
  private $db;
  public function __construct($host,$user,$pass,$db){
    $this->host = $host;
    $this->user = $user;
    $this->pass = $pass;
    $this->db = $db;
  }
  public function connect(){
    $this->connect_id = mysql_connect($this->host,$this->user,$this->pass);
    mysql_select_db($this->db);
    return $this->connect_id;
  }
  public function select($query, $ret_type = 2){
    $getData = mysql_query($query,$this->$connect_id);
    if( $getData == false )
      return false;
    switch( $ret_type ){
      case 1:
        while( $data = mysql_fetch_array($getData) )
          $this->ret_arr[] = $data;
        return $this->$ret_arr;
      case 2:
        while( $data = mysql_fetch_assoc($getData) )
          $this->ret_arr[] = $data;
        return $this->ret_arr;
      case 3:
        while( $data = mysql_fetch_object($getData) )
          $this->ret_arr[] = $data;
        return $this->ret_arr;
    }
  }
}
 
Versuche mal die Select Funktion noch wie folgt zu ändern:

PHP:
  public function select($query, $ret_type = 2){
    $getData = mysql_query($query,$this->$connect_id);
    if( $getData == false )
      return false;
    switch( $ret_type ){
      case 1:
        $fetch_func = 'mysql_fetch_array';
	  break;
      case 2:
        $fetch_func = 'mysql_fetch_assoc';
      break;
      case 3:
		$fetch_func = 'mysql_fetch_object';
	  break;
    }
	while( $data = $fetch_func($getData) )
	{
		$this->ret_arr[] = $data;
	}
	return $this->ret_arr;
  }

Dadurch sparst du dir den Code mehrmals zu wiederholen.

Was ist mit dem Auffangen von Fehlern?
Das geschieht in deiner Klasse noch überhaupt nicht.
 
Hi

das ist ja cool, ich wusste gar nicht, dass man das mit den Funktionsnamen so umschreiben kann, wieder was gelernt :) Zu dem Auffangen von Fehlern, meinst du in etwa so?
PHP:
<?php
class mysql{
  private connect_id;
  private $db;
  private $ret_arr;
  private $host;
  private $user;
  private $pass;
  private $db;
  private $error_msg = '';
  public function __construct($host,$user,$pass,$db){
    $this->host = $host;
    $this->user = $user;
    $this->pass = $pass;
    $this->db = $db;
  }
  public function connect(){
    $this->connect_id = mysql_connect($this->host,$this->user,$this->pass);
    $dbselect = mysql_select_db($this->db);
    if( $this->connect_id == false || $dbselect == false )
      $this->error_msg .= '<span style="color:#f00;">An error has occured: #'.mysql_errno().' ['.mysql_error().']</span><br />';
    return $this->connect_id;
  }
  public function select($query, $ret_type = 2){
    $getData = mysql_query($query,$this->$connect_id);
    if( $getData == false )
      $this->error_msg .= '<span style="color:#f00;">An error has occured: #'.mysql_errno().' ['.mysql_error().']</span><br />';
    if( $this->error_msg != '' ){
      echo $this->error_msg;
      return false;
    }
   $fetch_func = '';
    switch( $ret_type ){
      case 1: $fetch_func = 'mysql_fetch_array'; break;
      case 2: $fetch_func = 'mysql_fetch_assoc'; break;
      case 3: $fetch_func = 'mysql_fetch_object'; break;
      default:  $fetch_func = 'mysql_fetch_assoc'; break;
    }
    while( $data = $fetch_func($getData) )
        { $this->ret_arr[] = $data; }
    return $this->ret_arr;
  }
  public function clear_error_msg(){
    $this->error_msg = '';
    return;
  }
}
 
Ja und nein.

Ja du behandelst die Fehler, nein, nicht auf diese Weise.

In der objektorientierten Programmierung gibt es dafür eine tolle Art und Weise damit umzugehen:

Exceptions

Ich denke die werden dir gefallen, wenn man sich einmal reingearbeitet hat.
 
ich würde das erstellen der Verbindung in den constructer machen und das löschen der Verbindung in den destructor

den so ein Object instanziert man ja nur wenn man die Verbindung auch braucht
ich hatte auch mal sowas geschrieben magste mal sehn
 
Hi

ich bin jetzt soweit:
PHP:
<?php
class mysql{
  private connect_id;
  private $ret_arr;
  public function __construct($host,$user,$pass,$db){
    $this->connect_id = mysql_connect($this->host,$this->user,$this->pass);
    $dbselect = mysql_select_db($this->db);
    if( $this->connect_id == false || $dbselect == false )
      throw new Exception('<span style="color:#f00;">An error has occured: #'.mysql_errno().' ['.mysql_error().']</span><br />');
    return $this->connect_id;
  }
  public function __deconstruct(){
    mysql_close($this->connect_id);
  }
  public function select($query, $ret_type = 2){
    $getData = mysql_query($query,$this->$connect_id);
    if( $getData == false )
      throw new Exception('<span style="color:#f00;">An error has occured: #'.mysql_errno().' ['.mysql_error().']</span><br />');
    if( $this->error_msg != '' ){
      echo $this->error_msg;
      return false;
    }
    switch( $ret_type ){
      case 1: $fetch_func = 'mysql_fetch_array'; break;
      case 2: $fetch_func = 'mysql_fetch_assoc'; break;
      case 3: $fetch_func = 'mysql_fetch_object'; break;
    }
    while( $data = $fetch_func($getData) )
        { $this->ret_arr[] = $data; }
    return $this->ret_arr;
  }
}
$test1 = new mysql('localhost','foo','bar','db1');
try{
  $test1->connect();
} catch(Exception $e){
  echo $e->getMessage();
}
try $test1->select("SELECT * FROM tbl_name WHERE foo = $bar");
catch(Exception $e) echo $e->getMessage();

Kann ich die unteren Schreibweisen beide verwenden?
 
Zurück