Critique My Codeigniter Benutzerdefiniertes CMS-Seitenmodell

1432
Quelle (EN)
Dwayne Charrington

Ich entwickle derzeit ein benutzerdefiniertes CMS, das auf Codeigniter aufgebaut ist, und habe mich gefragt, ob Sie Fehler in meinem Modellcode für das Abrufen von Seiten finden können. Das Seitenabrufmodell ist nicht vollständig abgeschlossen, aber die Hauptfunktionalität zum Abrufen einer Seite sowie zum Abrufen von Modulen, die einer Seite zugewiesen sind (ein Modul ist wirklich nur ein Widget).

Kann dieses Modell in einigen Bereichen besser sein, vielleicht in Bezug auf die Verknüpfungen, die ich mache? Obwohl nicht wirklich Joins, sondern mehrere Abfragen, um Teile der zugehörigen Informationen wie Module und Medien abzurufen.

<?php

  class Mpages extends CI_Model {

      public function __construct()
      {
          parent::__construct();
      }

      public function fetch_page($page_slug = 'home') 
      {
          $db = $this->db;

          $query = 
            $db
               ->where('page_status', 1)
               ->where('page_slug', strtolower($page_slug))
               ->get('pages')
               ->result_array();

          $page_id = $query[0]['id'];

          $query['modules'] = 
            $db
                ->select('modules.module_name, modules.module_slug, modules.id moduleid')
                ->where('page_id', $page_id)
                ->join('pages_modules lpm', 'moduleid = lpm.module_id')
                ->order_by('module_order', 'asc')
                ->get('modules')
                ->result_array();

          /*$query['media'] = 
            $db
                ->select('lucifer_media.media_file_name, lucifer_media.media_file_extension, lucifer_media.media_directory')
                ->join('lucifer_pages_media', 'lucifer_pages_media.page_id = '.$page_id.'')
                ->get('lucifer_media')
                ->result_array();*/

          if ($query) {

            return $query;

          } else {
            return false;
          }
      }

      public function fetch_navigation()
      {
        $result = $this->db->order_by("nav_order", "asc")->where('published', 1)->get('navigation')->result_array();
        return $result;
      }

      public function fetch_layout($id)
      {

          $result = $this->db->where('id', $id)->get('layouts')->result_array();

          return $result[0];

      }

  }

?>
Antworten
6

2 Antworten auf die Frage

4
edorian

Ich kenne Codeigniter nicht, daher kann ich nicht kommentieren "Ist dies ein gutes Modell"? Für meine Antwort werde ich nur annehmen, dass es ist.

Es ist nicht viel Code, also werde ich mich auf einige Details konzentrieren: 

  public function __construct()
  {
      parent::__construct();
  }

Diese 5 Zeilen machen absolut nichts. Ich würde sie entfernen. Wenn Sie später einen Konstruktor benötigen, fügen Sie ihn später hinzu.

  public function fetch_navigation()
  {
    $result = $this->db->order_by("nav_order", "asc")->where('published', 1)->get('navigation')->result_array();
    return $result;
  }

Dies ist ziemlich schwer zu lesen (113 Zeichen in dieser Zeile). Ich würde für so etwas gehen (zu sagen mit Ihrer Formatierung von oben)

  public function fetch_navigation()
  {
      return $this->db
          ->order_by("nav_order", "asc")
          ->where('published', 1)
          ->get('navigation')
          ->result_array();
  }

$db = $this->db;

Es scheint nicht notwendig zu sein, eine lokale Variable zu erstellen, diese 6 zusätzlichen Zeichen sollten nicht schaden. Ich musste zweimal nachsehen, woher die Variable stammt und ob sie lokal ist, da Änderungen daran vorgenommen werden.

Bisher sind das wirklich kleine Noten.

Unerreichbarer Code ?

if ($query) {
  return $query;
} else {
  return false;
}

Sie setzen also $query['modules'] = ...so, auch wenn das null ist, wird die Abfrage mindestens enthalten

array("modules" => null);

und das wird immer wahr sein.

Ich freue mich, dass Sie sich die Zeit und Mühe genommen haben, um eine Lösung zu veröffentlichen. Was den Konstruktor anbelangt, müssen Sie dies mit dem übergeordneten Controller des Codeigniter-Frameworks verbinden. Andernfalls glaube ich, dass Sie keine vorinstallierten Bibliotheken, Modelle und andere automatisch im Kern geladene Objekte verwenden können. Gültiger Punkt über $ this-> db. Ich habe das schon immer gemacht, habe mich aber auch gefragt, ob die Zuweisung zu einer $ db-Variablen den zusätzlichen Aufwand wert war und komplizierte Dinge machte. Danke auch für den Nullpunkt der Module. Dwayne Charrington vor 10 Jahren 0
Ähm, $ db = & $ this-> db; `wäre vorteilhafter, wäre es nicht? RobertPitt vor 10 Jahren 0
@RobertPitt Das kann sogar schädlich sein. Objekte werden sowieso als Referenz übergeben und sind im Allgemeinen verletzend. Es gibt nur einige sehr spezielle Fälle, in denen es von Nutzen ist, sie zu nutzen edorian vor 10 Jahren 0
@Dwayne Wenn Sie die Zeilen einfach weglassen, wird der Konstruktor der übergeordneten Klasse von PHP aufgerufen. Das Einzige, was Sie nicht tun sollten, ist, es komplett leer zu lassen (deshalb mag ich es auch nicht, "immer einen Konstruktor zu haben", wenn jemand es leer lässt und Sachen kaputt macht edorian vor 10 Jahren 0
@edorian, ich glaube, dass Objekte erst ab PHP 5.2.0 als Referenz übergeben werden, da Codeigniter in CI 2.0 an der Unterstützung von PHP 4 festhält. Ich glaube, ich habe mich bei den Anwendungsstandards nur darauf beschränkt. RobertPitt vor 10 Jahren 0
@RobertPitt Juding von http://www.php.net/manual/de/language.oop5.references.php Dies geschah mit der PHP-Version 5.0 - ich habe keinen schnellen Zugriff auf eine wirklich alte 5.1.6, um sie zu testen aber ich bin mir ziemlich sicher :) Wenn nicht, lass es mich wissen edorian vor 10 Jahren 2
Ich entschuldige mich, ich hatte diese Informationen von einem Kommentar auf der PHP-Site erhalten, also hatte ich es speziell für diese Veröffentlichung gehalten. Nachdem ich einige Zend-Papiere gelesen hatte, kann ich sehen, dass es Teil der großen Version in 5.0 war. +1 RobertPitt vor 10 Jahren 0
Tolle Info Jungs, wirklich aufschlussreiches Zeug. Ich gab mir die Aufgabe, das $ this-> db einer $ db-Variablen zuzuweisen. Es schien mir nicht wert zu sein, um ehrlich zu sein. Ich möchte meinen Code so sauber und effizient wie möglich halten. Dwayne Charrington vor 10 Jahren 0
@Dwayne, Sauberhalten des Codes beginnt hier: http://framework.zend.com/manual/de/coding-standard.coding-style.html RobertPitt vor 10 Jahren 0
Danke für den Link Robert. Ich kenne den Zend-Standard für Codierstil und -konventionen, da ich ihn leider bei der Arbeit verwenden muss. Hier ist der Link zur Codeigniter-Dokumentation, nicht so lang, aber dennoch kurz: http://codeigniter.com/user_guide/general/styleguide.html Dwayne Charrington vor 10 Jahren 0
4
Zathrus Writer

aaaah, ein CodeIgniter-Kerl :-)

Ich arbeite gerade an einem CI-Projekt und habe bereits einige der Optimierungen implementiert, die Sie auch für Ihr CMS verwenden könnten.

  1. Um so wenig Aufwand wie möglich zu vermeiden, versuchen Sie, das verzögerte Laden Ihrer Dateien (Bibliotheken, Modelle ...) zu implementieren.

  2. Zum Zwischenspeichern können Sie KHCache verwenden - eine Bibliothek, mit der Sie Teile der Website anstelle einer vollständigen Seite zwischenspeichern können

  3. statt immer $ tun this-> db -> ..., können Sie eine erstellen Helferfunktion, zum Beispiel „Funktion _db ()“ und dann einfach tun _db ()-> wo ...

  4. Sie können optional auch eine Hilfsfunktion erstellen, um das Ergebnisfeld automatisch zu erhalten. Daher ist -> result_array () nicht mehr erforderlich: function res () {} ... $ query = res (_db ()-> where. ..);

jetzt zum Code :-)

$query = 
    $db
      ->where('page_status', 1)
      ->where('page_slug', strtolower($page_slug))
      ->get('pages')
      ->result_array();

$page_id = $query[0]['id'];

Anscheinend wählen Sie alle Werte aus der Datenbank aus, während Sie eine einzige erste ID benötigen. Versuchen Sie, die Anzahl der Ergebnisse zu begrenzen. Andernfalls wird in Ihrer Datenbank ein Overhead erzeugt

$db->where...->limit(1);

Die zweite Abfrage könnte wahrscheinlich ein LEFT JOIN anstelle eines regulären JOIN verwenden, obwohl ich Ihnen die Entscheidung überlasse (der JOIN-Ansatz führt möglicherweise nicht alles auf, was Sie benötigen).

$db-select...->join('pages_modules lpm', 'moduleid = lpm.module_id', 'left')

Ich denke, das ist alles ... Denken Sie daran, die richtigen Indizes für Ihre SQL-Felder zu verwenden und die EXPLAIN- Anweisung zu verwenden, um nach Engpässen zu suchen

Viel Glück!

und da ich aufgrund der schlechten Bewertung keine Links posten kann, sind sie hier - faule Last: http://codeigniter.com/forums/viewthread/134605/ Zathrus Writer vor 9 Jahren 0
Sie finden KHCache hier: http://codeigniter.com/forums/viewthread/69843/ Zathrus Writer vor 9 Jahren 0
und schließlich die EXPLAIN-Anweisung aus dem MySQL-Handbuch: http://dev.mysql.com/doc/refman/5.0/de/explain.html ... Ich würde mich freuen, wenn Sie diese Antwort akzeptieren würden, wenn Sie sie für nützlich erachten. so kann ich diese Link-Beschränkungen aufheben :-D Zathrus Writer vor 9 Jahren 0
statt einer Hilfsfunktion warum nicht `$ db = & $ this-> db` und dann` $ db-> select () ... ` RobertPitt vor 9 Jahren 0
Robert - um Zeit zu sparen ... _db () -> select ist schneller und erfordert nur eine Codezeile, während Sie, wie bei Ihrer Lösung, jedes Mal mehr Code des gleichen Codes schreiben müssen (was sich auch in der Zukunft ändern kann.) Sie müssen also in allen Dateien umfangreiche Suchen / Ersetzen durchführen.) Zathrus Writer vor 9 Jahren 0

Verwandte Probleme