Ankündigung

Einklappen
Keine Ankündigung bisher.

Klassendesign

Einklappen

Neue Werbung 2019

Einklappen
X
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Klassendesign

    Hallo Leute,

    ich arbeite im Moment an einem neuen, objektorientierten Ansatz für ein kleines Probe-CMS, das ich mir zur Aufgabe gemacht hab.
    Falls der Ansatz funktioniert will ich künftig alle meine Websites nach diesem Prinzip aufbauen.

    Ich würde dazu gern eine Frage stellen:
    Ich habe eine Klasse für eingehende Anfragen:

    PHP-Code:
    class Enquiry
    {
        private 
    $id;                // int
        
    private $mediumType;        // int / MediumType
        
    private $publicationDate;    // DateTime
        
    private $topic;                // String
        
    private $remarks;            // String
        
    private $attachment;        // String
        
    private $answeredPress;        // bool
        
    private $answeredFrom;        // String
        
    private $answerType;        // int / AnswerType
        
    private $depRedirect;        // String
        
    private $dirRedirect;        // String
        
    private $coopRating;        // String
        
    private $reportTerminated;    // bool
        
    private $dateCreated;        // DateTime
        
    private $creator;            // String --> Benutzertabelle???
        
    private $dateLastEdited;    // DateTime
        
    private $lastEditor;        // String --> Benutzertabelle???
        
    private $finished;            // bool
        
    private $deleted;            // bool
        
        
    public function __construct($id)
        {
            
    $dbh DB::getHandle();
            
    $row $dbh->query('SELECT * FROM `pm_new_enquiries` WHERE `id`='.$id)->fetch(PDO::FETCH_NUM);
            
            list(
                
    $this->id,
                
    $this->mediumType,
                
    $this->publicationDate,
                
    $this->topic,
                
    $this->remarks,
                
    $this->attachment,
                
    $this->answeredPress,
                
    $this->answeredFrom,
                
    $this->answerType,
                
    $this->depRedirect,
                
    $this->dirRedirect,
                
    $this->coopRating,
                
    $this->reportTerminated,
                
    $this->dateCreated,
                
    $this->creator,
                
    $this->dateLastEdited,
                
    $this->lastEditor,
                
    $this->finished,
                
    $this->deleted
            
    ) = $row;
        }
        
        public function 
    getId() { return $this->id; }
        public function 
    getMediumType() { return $this->id; }
        
    //...
        
        
    public function fetchList($start$length)
        {
            
    $ret = array();
            
    $dbh DB::getHandle();
            
    $sth $dbh->query('SELECT * FROM `pm_new_enquiries` LIMIT '.$start.','.$length);
            
            while (
    $row $sth->fetch(PDO::FETCH_NUM)) {
                list(
                    
    $this->id,
                    
    $this->mediumType,
                    
    $this->publicationDate,
                    
    $this->topic,
                    
    $this->remarks,
                    
    $this->attachment,
                    
    $this->answeredPress,
                    
    $this->answeredFrom,
                    
    $this->answerType,
                    
    $this->depRedirect,
                    
    $this->dirRedirect,
                    
    $this->coopRating,
                    
    $this->reportTerminated,
                    
    $this->dateCreated,
                    
    $this->creator,
                    
    $this->dateLastEdited,
                    
    $this->lastEditor,
                    
    $this->finished,
                    
    $this->deleted
                
    ) = $row;
                
                
    $ret[] = $this;
            }
            
            return 
    $ret;
        }

    Die Methode fetchList würde natürlich nur funktionieren, wenn ich zuerst eine leere Instanz von Enquiry erstellen würde und die Operation darauf ausführen würde.
    Das scheint mir kein sehr guter Stil zu sein.
    Mein zweiter Ansatz wäre gewesen, im Konstruktor alle Parameter, die in der Klasse vorhanden sind, zu übergeben. Dann könnte ich die fetchList-Methode als statisch deklarieren und es würde wieder um einiges besser aussehen.

    Jetzt würde ich halt gern wissen, ob dieser zweite Ansatz sinnvoll ist (immerhin werden ja so alle Variablen im Prinzip doppelt angelegt, was aber mit der momentanen Methode fetchList auch nicht anders wäre) oder ob beide Ansätze totaler Mist sind und ich mir lieber was anderes suchen sollte...

    Ich danke schonmal im Voraus für alle Antworten

    mfg SilentSight

  • #2
    Zitat von SilentSight Beitrag anzeigen
    Die Methode fetchList würde natürlich nur funktionieren, wenn ich zuerst eine leere Instanz von Enquiry erstellen würde und die Operation darauf ausführen würde.
    Das scheint mir kein sehr guter Stil zu sein.
    Das scheint ein ActiveRecord-Pattern zu werden oder?

    Demnach wäre, wenn überhaupt, das erzeugen einer Objektinstanz notwendig weil du sonst immer die gleiche Objektinstanz in deinem Array hast.

    Allerding sist der Ansatz: 1 Objekt = 1 Datenzeile. Methoden für die Manipulation oder Abfrage mehrerer Objekte würde ich in eine Klasse EnquiryManager auslagern.

    Zitat von SilentSight Beitrag anzeigen
    Mein zweiter Ansatz wäre gewesen, im Konstruktor alle Parameter, die in der Klasse vorhanden sind, zu übergeben. Dann könnte ich die fetchList-Methode als statisch deklarieren und es würde wieder um einiges besser aussehen.
    Und dein Konstruktor hat dann eine Parameterliste von hier bis nach Meppen? Davon abgesehen das du innerhalb einer statischen Methode sowieso keinen Zugriff auf nicht-statische Membervariablen hast womit sich das wohl erledigen würde.

    Eine Kombination aus beiden Ansätzen wäre aber denkbar. Also die fetchList() Methode wie oben benannt aber eben statisch, da keine Objektinstanz benötigt wird um sie auszuführen
    "Alles im Universum funktioniert, wenn du nur weißt wie du es anwenden musst".

    Kommentar


    • #3
      Das Pattern, was du bei fetchList versuchst anzuwenden, nennt sich afaik Prototype. Das funktioniert aber nur, wenn du den "Prototyp" klonst und nicht nur mehrere Referenzen auf das gleiche Object in das Array speicherst.

      Ansonsten hab ich es lieber, wenn Datenherkunft und Model getrennt sind.
      Wenn du es aber lieber so lassen willst, dann solltest du die Klasse lieber auf lazy-load umstellen, d.h., dass die eigenschaften erst geladen werden, wenn sie abgefragt werden und im konstruktor erstmal nur die ID speicherst.

      Kommentar


      • #4
        Hallo, erstmal zur Quellcodebewertung:

        - benutz protected statt private, so ermöglichst du eine Vererbung der Klasse. Das ist nicht immer notwendig, aber du schränkst dich auch nicht unnötig ein. private verwende ich fast nie, es sei denn ich weiß ganz genau, dass diese Eigenschaften/Methoden sehr speziell an die Klasse angepasst sind und von außen (also auch von erbenden Klassen) niemals geändert werden sollen, beispielsweise internes Caching o.ä.

        - sobald du anfängst Listen von Eigenschaften abzutippen (list ($this->id, ..)), solltest du dir über einen generischeren Ansatz Gedanken machen. Entweder lässt du dir so einen Code durch einen Quellcodegenerator erzeugen, der bei Änderungen der Datenbank deinen PHP-Code entsprechend anpasst, oder du legst die Eigenschaften als Array in der Klasse ab.

        - deine Klasse ist abhängig von der Datenbank und der Folge der Spalten. Wenn du zwischendurch eine Spalte in der Tabelle einfügst, wird dein PHP-Code nicht mehr funktionieren. Benutz statt FETCH_NUM entweder FETCH_ASSOC oder verzichte auf SELECT *-Abfragen.

        - im Konstruktor per Singleton eine DB-Instanz abzuholen ist schlecht. Übergib das Datenbank-Objekt per Konstruktor-Parameter

        - in fetchList erfolgt keine Validierung der Parameter, eine SQL-Injection ist möglich. Entweder du machst einen INT-Cast oder du benutzt mysql_real_escape_string().

        - du überschreibst in der while-Schleife deine Ergebnisse


        Grundsätzlich empfehle ich dir die Trennung von Datenobjekt und Datenquelle, wie auch schon vorgeschlagen. ActiveRecord ist für kleinere Projekte sicherlich praktisch und deshalb nicht pauschal zu verurteilen, allerdings wirst du bei vielen Querverbindungen zwischen den Objekten (User hat Profil, hat Nachrichten, hat Freunde, ..) schnell an deine Grenzen damit stossen. ActiveRecord eignet sich daher meiner Meinung nach nur für relativ unabhängige Objekte/Tabelleneinträge (z.B. Gästebücher o.ä.).
        "[URL="http://www.youtube.com/watch?v=yMAa_t9k2VA&feature=youtu.be&t=25s"]Mein Name ist Lohse, ich kaufe hier ein.[/URL]"

        Kommentar


        • #5
          Zitat von Chriz Beitrag anzeigen
          - in fetchList erfolgt keine Validierung der Parameter, eine SQL-Injection ist möglich. Entweder du machst einen INT-Cast oder du benutzt mysql_real_escape_string().
          bzw. (da du bereits PDO benutzt), Prepared Statements.

          Kommentar


          • #6
            @Dark Guardian:
            Richtig erkannt, das sollte ein ActiveRecord-Pattern werden. Ich hab das konzept vor ein paar Tagen im Type3-Framework Flow3 gefunden und habe es eigentlich recht elegant gefunden.
            Da ist ja auch das mit dem "Manager", nur eben als "Repository" beschrieben.
            Das mit der ewig langen Parameterliste hat mich eben auch gestört. Aber mal anders gefragt: Wie soll ich es sonst lösen, dass ich neue Anfragen hinzufüge? Ich muss immerhin die Werte irgendwie in die Klasse einbringen, ob ich die Parameter jetzt an den Konstruktor oder eine Methode "add" übergebe...

            @draco88:
            Leider verstehe ich nicht ganz was du mit der Trennung von Datenherkunft und -model meinst.
            Aber ich hab grad kapiert, dass die while-Schleife wirklich Schwachsinnig ist, ich speichere ja nicht das Objekt als Wert in's Array sondern bloß den Zeiger auf das Objekt, das hab ich wohl gestern übersehen...
            Und was LazyLoad angeht: Das ist doch im Prinzip das gleiche wie Singleton, oder verstehe ich das auch falsch???

            @Chriz
            - im Konstruktor per Singleton eine DB-Instanz abzuholen ist schlecht. Übergib das Datenbank-Objekt per Konstruktor-Parameter
            Wäre es wirklich so besser oder sollte ich die Klasse vielleicht sogar von der DB-Klasse ableiten?

            @Dormilich:
            Ja, sonst benutze ich auch Prepared Statements. Ich habs bloß gestern weggelassen (;

            Kommentar


            • #7
              Zitat von SilentSight Beitrag anzeigen
              @Dark Guardian:
              Richtig erkannt, das sollte ein ActiveRecord-Pattern werden. Ich hab das konzept vor ein paar Tagen im Type3-Framework Flow3 gefunden und habe es eigentlich recht elegant gefunden.
              Da ist ja auch das mit dem "Manager", nur eben als "Repository" beschrieben.
              Das mit der ewig langen Parameterliste hat mich eben auch gestört. Aber mal anders gefragt: Wie soll ich es sonst lösen, dass ich neue Anfragen hinzufüge? Ich muss immerhin die Werte irgendwie in die Klasse einbringen, ob ich die Parameter jetzt an den Konstruktor oder eine Methode "add" übergebe...
              Wie Chriz schon geschrieben hat solltest du einen generischen Ansatz verfolgen.

              Eine Möglichkeit wäre die Felder über magic getter/setter Methoden zu manipulieren und im Query mit den dort festgelegten Werten zu agieren.

              Z.B. so:

              PHP-Code:
              class ActiveRecord {
                 private 
              $fields arra;

                 public function 
              __get($f) {
                    return 
              $this->fields[$f];
                 }

                 public function 
              __set($f$v) {
                    
              $this->fields[$f] = $v;
                 }

                 public function 
              fillFields(Array $fields) {
                    
              $this->fields $fields;
                 }

                 public function 
              insert() {
                    
              $dbFields array_keys($this->fields);
                    
              $dbValues arra_values($this->fields);

                    
              $sql "INSERT INTO table ('".implode("','"$dbFields)."') VALUES ('".implode("','"$dbValues)."')";
                 }
              }

              $r = new ActiveRecord();
              $r->id '10';
              $r->field1 'value1';
              $r->field2 'value2';
              $r->insert(); 
              "Alles im Universum funktioniert, wenn du nur weißt wie du es anwenden musst".

              Kommentar


              • #8
                Von den magic gettern und settern hab ich auch schon gehört, aber mir wurde in der Berufsschule was von strikter Sicherheit in Klassen beigebracht. Beispielsweise soll die ID ja von außerhalb nicht geändert werden dürfen, was mit dieser Lösung aber möglich wäre... Außerdem habe ich gelesen, dass die magic setter und getter nicht wirklich das wahre im Hinblick auf die Performance sind, ob das wahr und inwiefern das wirklich relevant ist kann ich natürlich nicht sagen...
                Aber allein aus ersterem Grund muss ich diese Möglichkeit fast schon ausschließen.

                Mit "generischer Ansatz" meint ihr einen möglichst Allgemeinen? Das versuche ich ja hinzubekommen.

                Kommentar


                • #9
                  Zitat von SilentSight Beitrag anzeigen
                  @draco88:
                  Leider verstehe ich nicht ganz was du mit der Trennung von Datenherkunft und -model meinst.
                  Aber ich hab grad kapiert, dass die while-Schleife wirklich Schwachsinnig ist, ich speichere ja nicht das Objekt als Wert in's Array sondern bloß den Zeiger auf das Objekt, das hab ich wohl gestern übersehen...
                  Und was LazyLoad angeht: Das ist doch im Prinzip das gleiche wie Singleton, oder verstehe ich das auch falsch???
                  Du teilst deine Klasse in 2 oder meist noch mehr verschiedene Klassen auf.

                  Zum einen das Model, was im Grunde nur ein verbessertes Array ist und einfach nur Key/Value Paare speichert.
                  PHP-Code:
                  class Model {
                      protected 
                  $id;
                      
                  /* mehr attribute */
                      
                      
                  public function setId($id) {
                          
                  $this->id $id;
                      }
                      public function 
                  getId() {
                          return 
                  $this->id;
                      }
                      
                  /* mehr methoden */

                  und eine zum model passende Klasse, die die Daten aus der Datenbank liest und in ein Model schreibt, bzw aus dem Model liest und in die DB schreibt.

                  Kommentar


                  • #10
                    Sozusagen also ein Model Database Controller (um das mal mit dem MVC-Pattern zu vergleichen)?
                    Also Das Model, was meine Anfrage ist. Eine Klasse, die von der Haupt-Datenbank-Klasse erbt und die nötigen Daten für mein Model ausliest und eine Controller-Klasse, die die Daten zwischen diesen beiden Klassen austauscht?

                    Achja, bevor noch jemand was zu meinem Unverständnis der Unterschiede von Lazy-Loading und Singleton schreiben will, ich hab den Unterschied gefunden (;
                    Bei Singleton kann man ja nur eine Instanz der Klasse haben, aber bei Lazy-Loading wird bloß das Laden der Daten verzögert, aber mehrere Objekte können trotzdem erzeugt werden.

                    Kommentar


                    • #11
                      Zitat von SilentSight Beitrag anzeigen
                      Von den magic gettern und settern hab ich auch schon gehört, aber mir wurde in der Berufsschule was von strikter Sicherheit in Klassen beigebracht. Beispielsweise soll die ID ja von außerhalb nicht geändert werden dürfen, was mit dieser Lösung aber möglich wäre...
                      Ich persönlich sehe keinen Grund warum man die Id nicht von außen setzen können sollte.

                      Die Sicherheiten der Membervairablen von außen sollte auch in sich schlüssig sein.

                      Nur mal so als Beispiel: Wenn du einen Datensatz dublizieren willst machst du i.d.R. einen Select und danach einen Insert ohne den Primärschlüssel. Ohne ndas setzen/löschen der Id von außen bekommst du das aber nicht hin. Außer du speicherst die Id eben seperat und ignorierst bsie beim Insert. Was passiert aber bei Tabellen mit Primärschlüsseln die sich über mehr als eine Spalte erstrecken?

                      Nur um den Primärschlüssel nicht setzen zu können halte ich das für etwas viel Aufwand.
                      "Alles im Universum funktioniert, wenn du nur weißt wie du es anwenden musst".

                      Kommentar


                      • #12
                        So, ich habe jetzt mal aus der einen Klasse ein paar mehr gemacht und versucht die Idee mit Daten- und Modelltrennung umzusetzen.
                        Dabei ist folgendes herausgekommen:

                        PHP-Code:
                        class EnquiryModel
                        {
                            private 
                        $id;                // int
                            
                        private $mediumType;        // int / MediumType
                            
                        private $publicationDate;    // DateTime
                            //...
                            
                            
                        public function getId() { return $this->id; }
                            public function 
                        getMediumType() { return $this->mediumType; }
                            public function 
                        getPublicationDate() { return $this->publicationDate; }
                            
                        //...
                            
                            
                        public function fetchData($data)
                            {
                                list(
                                    
                        $this->id,
                                    
                        $this->mediumType,
                                    
                        $this->publicationDate,
                                    
                        //...
                                
                        ) = $data;
                            }
                        }

                        class 
                        EnquiryData extends Database
                        {
                            public function 
                        getData($id)
                            {
                                return 
                        $this->getHandle()->query('SELECT * FROM `pm_new_enquiries` WHERE `id`='.intval($id))->fetch(PDO::FETCH_NUM);
                            }
                            
                            public function 
                        getPage($pageLength$page)
                            {
                                return 
                        $this->getHandle()->query('SELECT * FROM `pm_new_enquiries` LIMIT '.$start.','.$length.' ORDER BY `id` DESC');
                            }
                        }

                        class 
                        EnquiryController
                        {
                            private 
                        $data;
                            public 
                        $model;
                            
                            
                            public function 
                        __construct($em$ed)
                            {
                                
                        $this->model $em;
                                
                        $this->data $ed;
                            }
                            
                            public function 
                        getEnquiryById($id)
                            {
                                
                        $this->model->fetchData($this->data->getData($id));
                            }
                            
                            public function 
                        getPage($pageLength$page)
                            {
                                
                        // Nicht gut...
                                
                        $em = new EnquiryManager();
                                
                        $em->getPage($this->data->getPage($pageLength$page));
                            }
                        }

                        class 
                        EnquiryManager
                        {
                            public function 
                        getPage($sth)
                            {
                                while (
                        $row $sth->fetch(PDO::FETCH_NUM)) {
                                    
                        $ret[] = new EnquiryModel();
                                    
                        // Hm... Wie weiter?
                                
                        }
                                
                                return 
                        $ret;
                            }
                        }

                        $model = new EnquiryModel();
                        $data = new EnquiryData();
                        $controller = new EnquiryController($model$data);
                        $controller->getEnquiryById(1);
                        $controller->model->getPublicationDate(); 
                        Ich bin mir nicht ganz sicher, ob das dem entspricht was ihr gemeint habt, deshalb würde ich ganz gern eure Meinung/Verbesserungsvorschläge dazu hören.
                        Und - falls es der richtige Weg ist - an zwei Stellen weiß ich momentan nicht weiter (sind kommentiert).

                        Ich freue mich natürlich weiterhin über jede Kritik (:

                        P.S. Das eine Attribut im Controller hab ich nur public gemacht, um zu schauen ob das alles überhaupt funktioniert.
                        Unter Umständen werde ich doch zu den Magic Gettern bzw. Settern wechseln (eine Überlegung ist es zumindest wert)...

                        Kommentar


                        • #13
                          Ein Beispiel aus der Zend Doku:

                          Code:
                          1. // application/models/Guestbook.php
                          2. class Application_Model_Guestbook
                          3. {
                          4. protected $_comment;
                          5. protected $_created;
                          6. protected $_email;
                          7. protected $_id;
                          8. public function __set($name, $value);
                          9. public function __get($name);
                          10. public function setComment($text);
                          11. public function getComment();
                          12. public function setEmail($email);
                          13. public function getEmail();
                          14. public function setCreated($ts);
                          15. public function getCreated();
                          16. public function setId($id);
                          17. public function getId();
                          18. }
                          19. class Application_Model_GuestbookMapper
                          20. {
                          21. public function save(Application_Model_Guestbook $guestbook);
                          22. public function find($id);
                          23. public function fetchAll();
                          24. }
                          Benutzt wird das ganze dann so:

                          Code:
                          1. $guestbook = new Application_Model_GuestbookMapper();
                          2. $entries= $guestbook->fetchAll();

                          Kommentar


                          • #14
                            Zitat von SilentSight Beitrag anzeigen
                            PHP-Code:
                            class EnquiryManager
                            {
                                public function 
                            getPage($sth)
                                {
                                    while (
                            $row $sth->fetch(PDO::FETCH_NUM)) {
                                        
                            $ret[] = new EnquiryModel();
                                        
                            // Hm... Wie weiter?
                                    
                            }
                                    
                                    return 
                            $ret;
                                }

                            hier kannst du die Iterierbarkeit von PDO ausnutzen …
                            PHP-Code:
                            class EnquiryManager
                            {
                                
                            # sicherstellen, daß du auch wirklich ein PDO resultset hast
                                
                            public function getPage(PDOStatement $sth)
                                {
                                    
                            $sth->setFetchMode(PDO::FETCH_NUM);
                                    foreach (
                            $sth as $row) {
                                        
                            $ret[] = new EnquiryModel();
                                        
                            // Hm... Wie weiter?
                                    
                            }
                                    return 
                            $ret;
                                }

                            falls du die Daten aus dem Query in EnquiryModel ablegen willst (da bin ich mir nich ganz sicher)
                            PHP-Code:
                            class EnquiryManager
                            {
                                
                            # sicherstellen, daß du auch wirklich ein PDO resultset hast
                                
                            public function getPage(PDOStatement $sth)
                                {
                                    
                            $sth->setFetchMode(PDO::FETCH_CLASS"EnquiryModel");
                                    foreach (
                            $sth as $row) {
                                        
                            $ret[] = $row;
                                    }
                                    return 
                            $ret;
                                    
                            # vielleich funktioniert auch
                                    
                            return $sth->fetchAll();
                                }

                            Kommentar

                            Lädt...
                            X