Ankündigung

Einklappen
Keine Ankündigung bisher.

Codeoptimierung:Code-Smells

Einklappen

Neue Werbung 2019

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

  • nikosch
    hat ein Thema erstellt Codeoptimierung:Code-Smells.

    Codeoptimierung:Code-Smells

    Diskussionsbeitrag zum Wiki Eintrag: Codeoptimierung:Code-Smells.

    Die Diskussionsplattform des PHP.de Wiki wurde ins Forum integriert. Durch Klicken des Buttons "Antwort" kannst du an diesem Thema teilnehmen.

  • Asterixus
    antwortet
    Unabhängig von Performance muss ich anmerken, dass nach der Codeübernahme einer anderen Person oder generell nach einiger Zeit Code mit select * total undurchsichtig ist. Es ist viel angenehmer, die Felder aufzulisten, da man somit sofort den Überblick hat, über was es gibt und was benötigt wird.
    Ich habe mich vor ein paar Tagen noch darüber aufgeregt, als ich das Projekt eines verschollenen Programmierers übernehmen musste.

    Tu deinem zukünftigen Dir oder einem potentiellen nächsten Programmierer den Gefallen und sorge dafür, dass dein Code klar ist.

    Einen Kommentar schreiben:


  • mermshaus
    antwortet
    (Sorry, das ist x-fach dasselbe, aber ich habe keine Lust, das noch mal pointierter zu formulieren.)

    Zitat von cyro Beitrag anzeigen
    Nein, das ist nicht mein Argument.

    Aber wenn man in einem Wikiartikel argumentiert, dann sollten die Argumente schon der Wahrheit entsprechen. Und das Folgefehler Argument hat halt nichts mit dem SELECT * zu tun und ist daher unangebracht. Ich denke ich hab das jetzt mehrfach schlüssig dargestellt.
    Die Folgefehler entstehen aber doch dadurch, dass SELECT * erst mal alles durchlässt und so unter Umständen Felder nicht gesetzt sind, die der Programmierer erwartet. Würden statt SELECT * alle Felder sauber in der Query aufgeführt, würde bei nachträglichen Änderungen am Datenbankschema bereits die Query selbst fehlschlagen und es gäbe erst mal keine Folgefehler.
    Wenn man dein Beispiel hernimmt:

    PHP-Code:
    function getUser($id) {
      
    // connect, variablenbehandlung, etc. 

      
    $ress mysql_query("SELECT `Id` , `Name` FROM Personen WHERE id=$id");
      if(
    $data mysql_fetch_assoc($ress)) {
        return 
    $data;
      }
      return 
    false;

    Wenn du in der Datenbank die Spalte „Name“ in „Nickname“ umbenennst, wird die Query sofort fehlschlagen.

    Darauf würdest du wieder sagen, dass es dann genauso Folgefehler geben könnte, denn der Programmierer könnte die Felder in der Query ändern, aber im nachfolgenden Code weiterhin falsche Feldnamen verwenden.

    Und darauf würde ich wieder sagen, dass so was in der Programmierung nie und bei gar nichts auszuschließen ist, was das meines Erachtens zu einem etwas müßigen Argument macht.

    Für mich steht zumindest fest, dass bei Abfragen ohne SELECT * ein weiterer Korrektheits-Check durchgeführt wird, der bei Abfragen mit SELECT * nicht durchgeführt wird.

    Ich zitiere noch mal den Abschnitt aus dem Wiki-Artikel:

    Zitat von Wiki-Artikel
    Zudem sagt das Statement nichts darüber aus, welche Werte es liefert. Kritisch wird es, wenn sich die Tabellenstruktur ändert - Folgefehler (Zugriff auf nicht mehr existente Feldnamen) oder das Auslesen von unnützen Daten (Text, Blob) kann die Folge der *-Konvention sein.
    Das meint eben einfach, dass eine Query mit SELECT * bei Änderungen an der Datenbank nicht fehlschlagen würde (was zu indirekten Folgefehlern führt), eine Query mit Auflistung der gewünschten Felder dagegen schon.

    Das ist ein zusätzlicher Nachteil von SELECT * und das macht Folgefehler wahrscheinlicher, weil ein zusätzlicher Korrektheits-Check fehlt. Wie wesentlich das ist, will ich nicht beurteilen, aber der Abschnitt ist so, wie er im Wiki steht, nicht falsch.

    Vielleicht noch gerade zu einem Absatz aus meinem vorherigen Post:

    Zitat von mermshaus
    Der Wiki-Artikel ist sicherlich nicht unbedingt für Profi-Entwickler gedacht, die ihren Code „vollständig“ mit Unit-Tests abgedeckt haben und die dazu noch eine Nachricht erhalten, falls in einer Anwendung mal irgendwo eine PHP Notice geworfen wird.
    Damit will ich sagen, dass es Leute, die SELECT * nutzen, unter Umständen überhaupt nicht merken, dass sie vergessen haben, ihren Code oder Teile ihres Codes an ein verändertes Datenbankschema anzupassen. Entweder haben sie überhaupt keine PHP-Notices aktiviert oder sie haben PHP-Notices zumindest auf dem Produktivsystem nicht aktiviert und testen lokal nicht jeden Aspekt des Codes vor Veröffentlichung. Das halte ich beides nicht für zu unwahrscheinlich.

    Wenn aber bereits die Query fehlschlägt, fällt das auf oder macht zumindest das vollständige Durchlaufen des Codes unmöglich. Dann wird nicht vergessen, die Mehrwertsteuer hinzuzurechnen oder dergleichen.

    PHP-Code:
    $price += $price $row['mwst']; 
    Es sei denn, der Programmierer ändert die Feldnamen in der Query, übersieht aber das 'mwst' an dieser Stelle. Dagegen ist aber wie gesagt überhaupt kein Kraut gewachsen und das ist vor allem nichts, was den zusätzlichen Nachteil von SELECT * aufhebt.

    Einen Kommentar schreiben:


  • tr0y
    antwortet
    * birgt aber die "Spekulative Allgemeinheit", man darf sich ruhig auf Folgefehler beziehen, wenn man alles bestellt, aber das was man im Endeffekt brauch nicht dabei sein kann. Genau genommen sollte die Fehlerquelle eher das Query sein, das Felder bestellt die nicht da sind, als das ein folgender Array-Zugriff auf ein Datenfeld zugreift, das garnicht mitkommt. Bei Query-Fehler, ist mitunter bei den neueren Datenbank-Interfaces auch besseres Fehler-Handling an Bord.

    Einen Kommentar schreiben:


  • cyro
    antwortet
    Na ja, dein Argument ist jetzt aber: „Es ist sinnfrei, auf die Korrektheit von Schnittstellenrückgaben zu achten, denn mich hält nichts davon ab, später im Code einen Tippfehler in einem Variablennamen zu haben.“
    Nein, das ist nicht mein Argument.

    Aber wenn man in einem Wikiartikel argumentiert, dann sollten die Argumente schon der Wahrheit entsprechen. Und das Folgefehler Argument hat halt nichts mit dem SELECT * zu tun und ist daher unangebracht. Ich denke ich hab das jetzt mehrfach schlüssig dargestellt.

    Einen Kommentar schreiben:


  • mermshaus
    antwortet
    Na ja, dein Argument ist jetzt aber: „Es ist sinnfrei, auf die Korrektheit von Schnittstellenrückgaben zu achten, denn mich hält nichts davon ab, später im Code einen Tippfehler in einem Variablennamen zu haben.“

    Ich weiß nicht, ob das so produktiv ist.



    Randaspekt: Der Wiki-Artikel ist sicherlich nicht unbedingt für Profi-Entwickler gedacht, die ihren Code „vollständig“ mit Unit-Tests abgedeckt haben und die dazu noch eine Nachricht erhalten, falls in einer Anwendung mal irgendwo eine PHP Notice geworfen wird.

    Einen Kommentar schreiben:


  • cyro
    antwortet
    Nagut, dann halt ein einfaches Beispiel:

    PHP-Code:
    function getUser($id) {
      
    // connect, variablenbehandlung, etc. 

      
    $ress mysql_query("SELECT `Id` , `Name` FROM Personen WHERE id=$id");
      if(
    $data mysql_fetch_assoc($ress)) {
        return 
    $data;
      }
      return 
    false;
    }

    // eigentlicher code
    // derartige zugriffe können vielfach in einem projekt verstreut sein
    if ($user getUser($id)) {
      echo 
    $user['login']; // BOOM

    Es spielt überhaupt keine Rolle, ob im SQL Statement ein SELECT * steht oder nicht. Der Folgefehler hat mit dem SQL Statement überhaupt nichts zu tun. Der Programmcode muss nicht immer in der While Schleife hinter der Datenbankabfrage stehen. Und selbst dann, muss der Zugriff auf die Variable auch bei einer expliziten Abfrage geändert werden. Der Verzicht auf SELECT * schützt einen vor den Folgefehlern nicht. Daher ist die Aussage mit den Folgefehlern falsch.

    Ein weiteres Argument gegen Select * ist definitiv der mögliche Speicherbedarf, wenn zum Beispiel Blob oder longtext-Spalten mit beteiligt sind - und auch noch mehr als 1 Datensatz ermittelt wird
    Sehe ich genauso. Wobei das eher von einem schlechten Datenbankdesign zeugt. Solche Fehler im Programmcode auszubügeln geht nur eine bestimmte Zeit lang gut.

    Einen Kommentar schreiben:


  • ChristianK
    antwortet
    Zitat von cyro Beitrag anzeigen
    Hi Tr0y,

    ich stelle nicht den Sinn oder Unsinn von SELECT * in Frage. Daher pflichte ich dem oberen Abschnitt deines Postings bei.

    Mir geht es lediglich um diesen Teil:


    Ein Programmabbruch wird nicht durch SELECT * verursacht. Ein Programmabbruch wird aber verusacht, wenn man im SQL Statement explizit Felder abfragt, die nicht existieren. Korrigiert man das SQL Statement, können die Folgefehler ebenso bei der expliziten Angabe der Felder auftreten. Ein Zugriff auf $row['foo'] oder $object->foo wird zu einer Warnung führen, ganz egal wie das SQL Statement aussieht.

    Das Problem SELECT * anzulasten ist quatsch, da es auch auftritt, wenn man kein SELECT * benutzt.
    Ach ja? Stell dir vor, du dividierst einen beliebigen Wert durch einen Wert, der du aus der DB holst. Jetzt kommt da aber nichts, weil das Feld nicht mehr da ist ( also (null) ). Da wären wir schon bei einem Fatal-Error: dividide by zero.

    Du bekommst also ein unerwartetes Verhalten und kannst nicht auf die möglicherweise entstehenden Fehler reagieren, weil du nicht weist, welche Fehler entstehen könnten.

    Wenn du jedoch Felder holst, die nicht da sind erhälst du einen erwarteten Fehler: column not found.

    Deshalb schon aus diesem Grund niemals * anwenden, weil du dann die Anwendung nicht mehr unter Kontrolle hast.

    Einen Kommentar schreiben:


  • beliar284
    antwortet
    Zitat von cyro Beitrag anzeigen
    Hi Tr0y,

    ich stelle nicht den Sinn oder Unsinn von SELECT * in Frage. Daher pflichte ich dem oberen Abschnitt deines Postings bei.

    Mir geht es lediglich um diesen Teil:


    Ein Programmabbruch wird nicht durch SELECT * verursacht. Ein Programmabbruch wird aber verusacht, wenn man im SQL Statement explizit Felder abfragt, die nicht existieren. Korrigiert man das SQL Statement, können die Folgefehler ebenso bei der expliziten Angabe der Felder auftreten. Ein Zugriff auf $row['foo'] oder $object->foo wird zu einer Warnung führen, ganz egal wie das SQL Statement aussieht.

    Das Problem SELECT * anzulasten ist quatsch, da es auch auftritt, wenn man kein SELECT * benutzt.
    Doch .. genau ist das ein Problem spezifisch von SELECT * .. hier WEISST du nämlich gar nicht, welche Spalten kommen (sollen) ... du bist "blind" der Datenbank ausgeliefert.
    Und beim Debuggen hängst du dann "warum zum Geier geht $row['foo'] nicht (mehr) die hat doch noch vorgestern (ja - vor dem herumschrauben an der Tabelle, seit dem foo eben `funktionsanalyseparameter` heißt) - in PHP siehst du dann eben undefined index - aber die Fehlermeldung der Datenbank wäre viel aussagekräftiger "you have an error in you SQL syntax , unknown column `foo` - da weißt du gleich, dass es daran hängen muss ... die PHP-Meldung bekommt man gerade als Änfänger im rohen Dutzend um die Ohren gehauen - mit exakt dem gleichen Fehlertext (weil das uralt-Tutorial sich vielleicht auf register globals=on berufen hat )...da ist schon viel geholfen, wenn du die Quelle dieses Fehlers exakt kennst

    @hausl ... nicht wirklich .. der Parser muss so oder so schauen, welche Spalten da sind, um sie dann mit der Liste abzugleichen, die du beim Select angibst - oder eben nicht ...die Auswirkung dürfte eher im Bereich der Messbarkeitsgrenze liegen, zumal der Parser mit das best optimierte an Datenbanken ist

    Ein weiteres Argument gegen Select * ist definitiv der mögliche Speicherbedarf, wenn zum Beispiel Blob oder longtext-Spalten mit beteiligt sind - und auch noch mehr als 1 Datensatz ermittelt wird

    Einen Kommentar schreiben:


  • hausl
    antwortet
    Ich habe auch mal wo gelesen das es unperformanter ist weil MySQL "intern" bei einem SELECT * zuerst "schauen" muss was es gibt und dann abholt, bei einer definierten Anforderung aber gleich direkt abholt. Das hab ich aber nie genau hinterfragt und/oder ge-benchmarkt, weil ich sowieso nie anders mache...

    Abgesehen davon hast du ev. 100 Spalten/Felder und brauchst nur 10, im Result das du dann erhältst (also die DB-Daten-Array) hast du dann halt die volle Latte drinnen, was ev. auch wieder auf "Performance" (in dem Fall halt Memory) sich auswirkt.

    Falls dich das Argument Performance interessiert findest du sicher einiges dazu via Tante G.

    Einen Kommentar schreiben:


  • cyro
    antwortet
    Hi Tr0y,

    ich stelle nicht den Sinn oder Unsinn von SELECT * in Frage. Daher pflichte ich dem oberen Abschnitt deines Postings bei.

    Mir geht es lediglich um diesen Teil:
    Aus Entwicklungstechnischer sicht ist es unsauber alles via * zu selektieren, da du dich so blind auf die Datenbank-Struktur verlässt, was bei minimalen Änderungen zum Programmabbruch führen könnte, da plötzlich Felder nicht mehr da sind, die mal da waren.
    Ein Programmabbruch wird nicht durch SELECT * verursacht. Ein Programmabbruch wird aber verusacht, wenn man im SQL Statement explizit Felder abfragt, die nicht existieren. Korrigiert man das SQL Statement, können die Folgefehler ebenso bei der expliziten Angabe der Felder auftreten. Ein Zugriff auf $row['foo'] oder $object->foo wird zu einer Warnung führen, ganz egal wie das SQL Statement aussieht.

    Das Problem SELECT * anzulasten ist quatsch, da es auch auftritt, wenn man kein SELECT * benutzt.

    Einen Kommentar schreiben:


  • tr0y
    antwortet
    Man sollte auf den "all"-Select verzichten und explizit angeben welche Felder man haben möchte damit man in erster Linie weiß was wo angefordert wird und man es leicht(er) anpassen kann. Es ist eigentlich schiere Faulheit die gewünschten Felder nicht aufzuführen. Bei komplexeren Queries wirst du auch Feld-Aliase nutzen, damit du bei Assoziativen Arrays oder Object-Injection direkter arbeiten kannst, das ist allerdings nicht mit dem All-Fields-Selector (*) möglich.

    Anwendungen die Zentralisiert ihre Queries speichern ( bspw. in YAML-Files, wie ich das tuhe, wodurch ich über einen Pimple-Container entsprechend auf die Queries zugreif.. ), abstrahieren ihre Queries mitunter ( wenns kein ORM-System oder andere "Helfer" nutzen ) sodass Teile von Queries entsprechend auch wiederverwendet werden können.

    Code:
    ---
    
    # base query
    foo: *foo |
    SELECT
       a,
       b,
       c,
       d,
       e,
    FROM foobar;
    
    foobar: # will be auto-join(' ', therms)'ed within container mechanics
    
       &foo
    
       -: |
          WHERE a = :a LIMIT 12
    
    bazbar: # will be auto-join(' ', therms)'ed within container mechanics
    
       &foo
    
       -: |
          WHERE b = :b LIMIT 100
    PHP-Code:
    $prep $app['database']->prepare($app['queries']['bazbar']);
    /* ... */ 
    Aus Entwicklungstechnischer sicht ist es unsauber alles via * zu selektieren, da du dich so blind auf die Datenbank-Struktur verlässt, was bei minimalen Änderungen zum Programmabbruch führen könnte, da plötzlich Felder nicht mehr da sind, die mal da waren.

    Einen Kommentar schreiben:


  • cyro
    antwortet
    OK, danke. Ich verstehe jetzt wie es gemeint ist. Allerdings empfinde ich das Argument konstruiert. Man erhält in diesem Fall eine Undefined index: foo Warnung, die ganz genau Auskunft über das Problem gibt.

    Selbst wenn man die Datenbankänderung vergessen hat oder sie von jemand anderem durchgeführt wurde, so wird man nach der Änderung auch die SQL Statements anpassen müssen. Sobald die geändert sind, unterscheidet sich die Situation aber nicht mehr von einem SELECT *. Wenn irgendwo im Code auf $row['foo'] zugegriffen wird, wird eine Warnung generiert und es macht dann überhaupt keinen Unterschied wie das SQL Statement aussieht.

    Einen Kommentar schreiben:


  • hausl
    antwortet
    Ich nachfolgenden Code.. du machst $row['foo'] und weiß nicht (mehr) das es foo nicht mehr gibt, findest es aber erst nach einigem debuggen raus das der Ursprung des Bösen ist das es das "oben" nicht (mehr) gibt.

    Einen Kommentar schreiben:


  • cyro
    antwortet
    SELECT *:
    ... Folgefehler (Zugriff auf nicht mehr existente Feldnamen) ...
    Das verstehe ich nicht. Wie kann man mit SELECT * auf nicht mehr existierende Feldnamen zugreifen? Das kann doch nur dann passieren, wenn man kein SELECT * verwendet. Oder was ist mit der Aussage gemeint?

    Edit:
    Ich hab mir jetzt auch mal den Rest angesehen und dabei ist mir auch "LIMIT und Schleife" aufgefallen. Ein LIMIT 1 bei einer Abfrage auf einen Primärschlüssel ergibt für mich ebenfalls keinen Sinn. Das ist unnötig, weil es nur ein Ergebnis geben kann. Damit kaschiert man höchsten erhebliche Mängel im Datenbankdesign.

    Einen Kommentar schreiben:

Lädt...
X