Code verkürzen?

[Livingo]

PHP Code:

class datsql {
        public static $connection;
        public static $isConnected;
        public static $cacheQuery;
        public static $cachePrepare;
        public static $pdoCount;
        public static $count = 0;
        
        public function reset($var) { $var = NULL; }
        public function error($e, $cust) { if(isset($cust)) { print 'Error: '. $cust; } else { print 'Error: '. $e->getMessage(); } self::$isConnected = false; die(); }
        public function __construct() {
            global $dfHost, $dfUser, $dfPassword, $dfDatabase, $dfCustomPort;
            try {
                self::$connection = new PDO('mysql:host='.$dfHost.';dbname='.$dfDatabase.';', $dfUser, $dfPassword);
                self::$isConnected = true;
            } catch (PDOException $e) {
                self::error($e);
            }
        }
        
        public function update($table, $column, $value, $where, $is) {
        if(empty($table) AND empty($column) AND empty($value)) { self::error(false, 'check {update}'); }
        
            if(self::$isConnected) {
                if(is_array($column) OR is_array($value)) {
                    if(is_array($column) AND is_array($value)) {
                        self::$cacheQuery = array();
                        foreach($column as $col) {                    
                            if($where) {
                                self::$cacheQuery[] = 'UPDATE '.$table.' SET '.htmlentities($col, ENT_QUOTES).'="'.htmlentities($value[self::$count]).'" WHERE '.$where.'="'.$is.'"';
                            } else {
                                self::error(false, 'illegal?'); // disabled
                                self::$cacheQuery[] = 'UPDATE '.$table.' SET '.htmlentities($col, ENT_QUOTES).'="'.htmlentities($value[self::$count]).'"';
                            }
                            self::$count++;
                        }
                        
                        foreach(self::$cacheQuery as $key => $query) {
                            try {
                                self::$cachePrepare    = self::$connection->prepare($query);
                                self::$cachePrepare->execute(); 
                            } catch (PDOException $e) {
                                self::error($e); 
                            }
                            self::reset(self::$count); 
                            self::reset(self::$cachePrepare);
                            self::reset(self::$cacheQuery);
                        }
                        
                    } else {
                        if(is_array($value)) { self::error(false, 'good joke'); } elseif(is_array($column)) {
                            self::$cacheQuery = array(); 
                            foreach($column as $col) {
                                if($where) {
                                    self::$cacheQuery[] = 'UPDATE '.$table.' SET '.htmlentities($col, ENT_QUOTES).'="'.$value.'" WHERE '.$where.'="'.$is.'"';
                                } else {
                                    self::error(false, 'illegal?'); // disabled
                                    self::$cacheQuery[] = 'UPDATE '.$table.' SET '.htmlentities($col, ENT_QUOTES).'="'.$value.'"';
                                }
                            }

                            foreach(self::$cacheQuery as $key => $query) {
                                try {
                                    self::$cachePrepare = self::$connection->prepare($query); 
                                    self::$cachePrepare->execute();
                                } catch (PDOException $e) {
                                    self::error($e); 
                                }
                                
                                self::reset(self::$cachePrepare);
                                self::reset(self::$cacheQuery);
                            }
                        } else { self::error(false, 'illegal?'); }
                    }
                } else {
                    try {
                        self::$cacheQuery = 'UPDATE '.$table.' SET '.htmlentities($column, ENT_QUOTES).'="'.htmlentities($value, ENT_QUOTES).'" WHERE '.$where.'="'.$is.'"';
                        self::$cachePrepare = self::$connection->prepare(self::$cacheQuery); 
                        self::$cachePrepare->execute();
                        self::reset(self::$cacheQuery);
                        self::reset(self::$cachePrepare); 
                    } catch(PDOException $e) {
                        self::error($e); 
                    }
                }
            }
        }
    } 


Hallo,

Habe mal eine Frage ist die Update-Funktion unnötig in die länge gezogen bzw. kann man Sachen simpler machen oder ist es gar kompletter Bullshit?

Gruß

[Mikesch01]

Hallo,

ich persönlich würde für die jeweiligen Statements keine eigene Methode schreiben, sondern eine gesamtheitliche Methode, die ein definiertes Statement dann ausführt.

Z.B:

PHP Code:

<?php
class datsql {
        public static $connection;
        public static $isConnected;
        public static $cacheQuery;
        public static $cachePrepare;
        public static $pdoCount;
        public static $count = 0;
        
        /* .... */
        
        public function query($sql) {
          /* ... bla dein Code */
          $stmt = self::$connection->query($sql);
          return $stmt;
        }
}
?>

Das ist jetzt nur ungefähr angegeben.

[dowhile]

1. Entweder du machst die Funktionen auch static und ersetzt den Konstruktor durch eine statische initialize() Methode oder du nutzt nicht static Attribute. Aber es macht keinen Sinn eine Klasse mit mehreren Objekten zu haben, wenn sich alle Objekte den selben Status teilen.

2. Greife die Variablen im Konstruktor nicht mit global ab sondern übergebe sie als Parameter. Ich sehe in deiner jetzigen Vorgehensweise nur Nachteile: Woher soll ein anderer Entwickler wissen, welche globalen Variablen er setzen muss, ehe er ein Objekt deiner Klasse erstellen darf? Oder besser: Niemand rechnet damit. Ein anderer Entwickler würde den Konstruktor einfach ohne globalen Variablen und ohne Parameter aufrufen und sich dann wundern, wieso das nicht funktioniert. (Und dieser andere Entwickler könntest auch du in zwei Monaten sein ...)

3. Anstatt deiner error()-Methode kannst du auch einfach eine Exception werfen (und fange die PDOException lieber nicht. Das ist nämlich so gedacht, dass man Exceptions erst dann fängt, wenn man sie auch sinnvoll behandeln kann. Einfach ausgeben und das Programm beenden ist in den meisten Fällen keine gute Art und Weise, damit umzugehen: Was ist, wenn du irgendwann im Programm ein Query ausführen möchtest, du aber unbedingt noch Aufräumarbeiten durchführen musst, wenn es nicht funktioniert? Zudem werden ungefangene Exceptions bei aktivierter Fehlerausgabe ohnehin ausgegeben).

4. Mit einem UPDATE Statement kannst du auch mehrere Spalten verändern (das ist auch schneller, als wenn du direkt hintereinander für jede Spalte ein eigenes Query raus gibst).

5. Ich würde wie Mickesch01 das Query auch nicht generieren lassen, denn Queries sind einfach zu komplex als könnte man jeden Fall abdecken. Selbst große ORMs wie Hibernate oder Doctrine bieten eine eigene Query-Language an.

Du siehst das bei dir auch: Obwohl deine update()-Methode noch lange lange nicht alles möglich macht, was bei einem UPDATE eigentlich möglich wäre, wird sie schon recht lange. Zum Beispiel ist dein WHERE Teil extrem eingeschränkt (AND, OR, >, <, usw?).

Andere Frage dazu: Wird deine Anwendung wirklich so groß, dass du irgendwann vielleicht einmal das Datenbanksystem austauschen müsstest?
Wenn nicht ändern sich die Queries ohnehin nicht und du musst sie nicht abstrahieren (ob nun durch Builder oder einer eigenen Syntax dafür).
Wenn doch würde ich auf einen fertigen ORM setzen (wie zum Beispiel Doctrine), denn damit sparst du dir sehr viel Arbeit.

6. Eigentliche Frage, verkürzen: Du hast zwei Zweige, nämlich wenn $column und $value jeweils Arrays sind und wenn nicht. Das kannst du zusammenfassen indem du am Anfang einfach immer eins daraus machst (also wenn eines von beiden kein Array ist einfach ein Array mit dem Wert als einziges Element erstellen). Dann hast du nur noch einen Zweig und sparst Code.