Ankündigung

Einklappen
Keine Ankündigung bisher.

PHP Code so ok?!

Einklappen

Neue Werbung 2019

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

  • PHP Code so ok?!

    Ich habe heute einmal versucht ein wenig mit PHP umzubasteln. Nun würde ich gern wissen, ob der Code so ok ist? Oder gibt es elegantere Lösungen?!

    Danke schonmal für Hinweise.

    Hier nun der Code:

    PHP-Code:
    <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
    <html>
    <body>
    <?php
            
    if(!isset($id))

        {
            print(
    "    <h1>Welche User soll gelöscht werden?</h1>");
            
            print(
    "<form action='deleteuser.php' method='post'>

    "
    );
            print(
    "<input type='hidden' name='id' value='true'>");
            print(
    "Benutzername:
    "
    );
            print(
    "<select name='login' size='1'>");
            print(
    "<option selected>--------------</option> ");
            
                
    $db=mysql_connect("localhost""root""");      
                
    //    or die ("Keine Verbindung zum Datenbankserver!");
                   
    mysql_select_db("museum");                        
                   
    //    or die ("Die Datenbank kann nicht angesprochen werden!");
                   
    $anfrage="SELECT `Login` FROM `benutzer`";               
                   
    $ergebnis=mysql_query($anfrage);
                   
    //    or die ("Fehler bei der Datenbakabfrage!");
                   
    $anz=mysql_num_rows($ergebnis);
                   for(
    $a=0;$a<$anz;$a++)
                       {    
    $nn mysql_result($ergebnis$a);
                          print(
    "<option>$nn</option>");
                       }    
                   
    mysql_close($db);
        
            print(
    "</select></p>");
            print(
    "



    "
    );
            print(
    "<input type='submit' value=' Absenden '>");
            print(
    "<input type='reset' value=' Abbrechen'>");
            print(
    "</form>");
        }
        
        
        
    else
        
        {        print(
    "Test erfolgreich
    "
    );
                
                
    $login=$_POST['login'];
                    
                
    $db=mysql_connect("localhost""root""");      
                
    //    or die ("Keine Verbindung zum Datenbankserver!");
                   
    mysql_select_db("museum");                        
                   
    //    or die ("Die Datenbank kann nicht angesprochen werden!");

                   
    $s="SELECT `Login` FROM `benutzer` WHERE `Login` LIKE '";
                   
    $s.=$login;
                
    $s.="'"
                            
                   
    $ergebnis=mysql_query($s);
                   
    //    or die ("Fehler bei der Datenbakabfrage!");
                   
    $anz=mysql_num_rows($ergebnis);
                   
                   print(
    "Sicherheitscheck - Anzahl der gefunden Datensätze: ");
                   print(
    $anz); 
                   
                   
                  if (
    $anz=1)
                          {    
                           
    $anfrage="DELETE FROM `benutzer` WHERE `Login` = '";
                        
    $anfrage.=$login;
                        
    $anfrage.="'"
                        
    $ergebnis=mysql_query($anfrage);
                               
                    }
                
                   else
                       {    print(
    "Löschen nicht möglich");
                       }    
                       
                   
    mysql_close($db);
            }                   
    ?>
    </body>
    </html>


  • #2
    ich würde statt print() den gebräuchlicheren befehl echo verwenden

    Kommentar


    • #3
      OK, daaanke.

      Noch jemand nen Rat?!?!

      Kommentar


      • #4
        PHP-Code:
                    $ergebnis=mysql_query("SELECT `Login` FROM `benutzer`");
                    while(
        $row mysql_fetch_array($ergebnis) {
                      echo
        "<option>$row['spaltenname']</option>";
                    } 
        so würde ich es lösen. Ist viel einfacher und mysql_fetch_array(); ist um einiges schneller als mysql_result();

        Kommentar


        • #5
          Die ganzen prints solltest du zusammenfassen:
          PHP-Code:
          <?
                echo " <h1>Welche User soll gelöscht werden?</h1>\n
                           <form action='deleteuser.php' method='post'>

          \n
                             <input type='hidden' name='id' value='true'>\n
                              Benutzername:
          \n
                            <select name='login' size='1'>\n
                              <option selected>--------------</option>\n";
          ?>
          Wenn du dir mal den HTML-Code im Browser ansiehst, wirst du feststellen, dass du eine kilometerlange Zeile produzierst, deshalb solltest du \n öfters einsetzen. Außerdem ist
          PHP-Code:
          if(!isset($id)) 
          falsch, schreibe
          PHP-Code:
          if(!isset($_POST['id'])) 
          Gruß
          phpfan

          Kommentar


          • #6
            PHP-Code:
            <?php
              $db
            =mysql_connect("localhost""root""")or die ('Verbinden: '.mysql_error());
            ?>
            Bei den Anderen genauso.
            PHP-Code:
            <?php
            $login
            =$_POST['login'];
            $s="SELECT `Login` FROM `benutzer` WHERE `Login` LIKE '";
            $s.=$login
            ?>
            Niemals Daten ungeprüft in die DB packen.
            mysql_real_escape_string()
            Diese Erweiterung ist EXPERIMENTELL.
            [...]
            Seien Sie gewarnt und verwenden Sie diese Erweiterung auf eigenes Risiko..

            Kommentar


            • #7
              geht dein script? is es sicher un zuverläßíg? wenn ja is es okay.

              Kommentar


              • #8
                Hallo.

                Falsch ist noch das $anz=1. Hier setzt du $anz auf 1, anstatt den Wert von $anz mit 1 zu vergleichen.

                Nicht falsch, aber unschön ist es, auf die externen Variablen ($_POST['...']) zuzugreifen, ohne deren Existenz vorher zu prüfen. Besser:
                PHP-Code:
                <?php
                $login 
                '';
                if (isset(
                $_POST['logn']))
                    
                $login $_POST['login'];

                // oder:

                $login = isset($_POST['login']) ? $_POST['login'] : '';
                ?>
                Weiterhin unschön ist, dass du deinen Code nicht konsequent Einrückst und das HTML, das du ausgibst.

                Und, eleganter geht es natürlich auch, aber dazu vielleicht an einem anderen Abend mehr...

                Basti

                Kommentar


                • #9
                  Zitat von vocken91
                  ich würde statt print() den gebräuchlicheren befehl echo verwenden
                  kannst du deine aussage auch begründen, außer, dass echo öfter benutzt wird?

                  Kommentar


                  • #10
                    Hier was zum Thema:

                    http://www.faqts.com/knowledge_base/...l/aid/1/fid/40
                    http://www.php-faq.de/q/q-string-print.html

                    Ich wüsste weder, warum echo gebräuchlicher sein soll, noch, warum man es hier verwenden sollte. Es ist Hacke wie Jose - allerdings würde ich mir die Klammern sparen.

                    Basti

                    Kommentar


                    • #11
                      Vielen Dank für die Tipps. Ich werde sie mir gleich mal zu Gemüte führen ...

                      Ich habe noch eine Frage:
                      In meinem PHP Buch wird bei einem Formularfeld immer auf eine andere PHP Datei verwiesen. Ist dann meine Umsetzung, alles in eine Datei zu schrieben, sinnvoll?! Oder sollte das lieber getrennt sein?
                      Für den Fall, dass meine Lösung so ok ist: Ist die Verwenung von ID und isset auch sinnvoll? Oder kann man das noch "eleganter" lösen?

                      Daaaanke schonmal für Antwort und Hilfe!!

                      Kommentar


                      • #12
                        Zitat von Basti
                        Hallo.

                        Weiterhin unschön ist, dass du deinen Code nicht konsequent Einrückst und das HTML, das du ausgibst.

                        Und, eleganter geht es natürlich auch, aber dazu vielleicht an einem anderen Abend mehr...

                        Basti
                        Ja, die HTML ist noch nicht schön .. aber das soll erstmal seitens PHP funktionieren und auch "elegant" gelöst sein. Das Bunte drum rum, kommt später mal .. :wink:

                        Und Code einrücken?!? Wie mach ich das am besten?!
                        Ich bin Angänger und mache es eigentlich so, dass es mirgut gefällt und ich damit klarkomme.

                        Kommentar


                        • #13
                          Ich nehm immer 4 leerstellen wenn was neues kommt also z.B. so:

                          PHP-Code:
                          <?php
                              
                          echo "bla";
                              if(
                          $hans == 2) {
                                  print 
                          'test is 2';
                              }
                              echo 
                          "usw...\n";

                          ?>
                          also nach { (if, switch, for, etc.) welches hinter der function steht neue Zeile mit 4 leerstellen mehr als in der davor.

                          Ich hoff ich drück mich richtig aus

                          Zu deinem Code noch, du sprichst 2mal die Datenbank an. Einmal im If than und einmal im else teil. Wenn du es nur einmal vor dem If machst, dann vermeidest du redundanzen (wiederholungen) und somit wird der Quelltext wiederrum ein wenig übersichtlicher.

                          Ausserdem würde ich anstatt print "<input type='button'>" die ' und " anders rum schreiben. Also print '<input type="button">'. Aber ich glaub ist beides erlaubt.

                          ______________________
                          edit
                          in deim Fall so

                          PHP-Code:
                          <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
                          <html>
                          <body>

                          <?php   

                              $db 
                          mysql_connect("localhost""root""") OR die ("Keine Verbindung zum Datenbankserver!");
                              
                          mysql_select_db("museum") OR die ("Die Datenbank kann nicht angesprochen werden!");
                           
                              if(!isset(
                          $_GET['id'])){
                                  print 
                          "<h1>Welche User soll gelöscht werden?</h1>\n";
                                  print 
                          '<form action="deleteuser.php" method="post">

                          '
                          ."\n";
                                  print 
                          '  <input type="hidden" name="id" value="true">'."\n";
                                  print 
                          "  Benutzername: 
                          \n"
                          ;
                                  print 
                          '  <select name="login" size="1">'."\n";
                                  print 
                          "    <option selected>--------------</option>";

                                  
                          $anfrage  "SELECT 
                                                   Login
                                               FROM 
                                                   benutzer;"
                          ;            
                                  
                          $ergebnis mysql_query($anfrage) OR die ("Fehler bei der Datenbakabfrage!");
                                  
                          $anz mysql_num_rows($ergebnis);
                                  for(
                          $a=0;$a<$anz;$a++) {
                                      
                          $nn mysql_result($ergebnis$a);
                                      print 
                          "    <option>$nn</option>";
                                  }   
                                  
                          mysql_close($db); // braucht man glaub ich nicht wirklich

                                  
                          print "  </select></p>";
                                  print 
                          "



                          \n"
                          ;
                                  print 
                          '  <input type="submit" value=" Absenden ">'."\n";
                                  print 
                          '  <input type="reset" value=" Abbrechen ">'."\n";
                                  print 
                          "</form>\n");
                              } else {
                                  print 
                          "Test erfolgreich 
                          \n"
                          ;

                                  
                          $s  "SELECT 
                                             Login 
                                         FROM 
                                             benutzer 
                                         WHERE 
                                             Login 
                                         LIKE '"
                          $_POST['login'] ."';";
                                             
                                  
                          $ergebnis mysql_query($s) OR die ("Fehler bei der Datenbakabfrage!\n");
                                  
                          $anz mysql_num_rows($ergebnis);
                                      
                                  print 
                          "Sicherheitscheck - Anzahl der gefunden Datens&auml;tze:\n";
                                  print 
                          $anz
                                       
                                  if (
                          $anz 1) {   
                                      
                          $anfrage "DELETE FROM 
                                                      benutzer
                                                  WHERE 
                                                      Login = '"
                          $_POST['login'] ."';"
                                      
                          $ergebnis=mysql_query($anfrage);
                                  } else {
                                      print 
                          "L&ouml;schen nicht m&ouml;glich\n" ;
                                  }   
                                  
                          mysql_close($db);
                              }               
                          ?>
                          </body>
                          </html>
                          Ich hoff ich hab alles geändert und keine Fehler eingebaut aber is ja nnur als Beispiel damit du es verstehst.

                          MFG

                          Kommentar


                          • #14
                            Zitat von Gertrud
                            In meinem PHP Buch wird bei einem Formularfeld immer auf eine andere PHP Datei verwiesen. Ist dann meine Umsetzung, alles in eine Datei zu schrieben, sinnvoll?! Oder sollte das lieber getrennt sein?
                            In der Regel ist der erste Schritt fü besseren Code eben die Aufteilung von Code in verschiene Dateien. Beispiel:
                            • alle Anfragen der Anwendung/Site gehen an index.php;
                              Hier wird alles erledigt, was jeden Aufruf betrifft;
                            • dort wird eine zentrale Konfigurations-Datei eingebunden - in deinem Fall würde die einfach die Datenbank-Verbindungsdaten enthalten;
                            • In der index.php wird die Anfrage analysiert und z.B. eine Datei action/deluser.php oder action/show_deluser_fom.php (also die auszuführende Aktion) eingebunden.
                            • Diese Action-Dateien analysieren die übergebenen Daten und führen die nötigen Aktionen aus, um a) zu entscheiden, welche "Seite", besser "View" der Benutzer ausgeliefert bekommen soll und b) alle Daten, die in diese Sicht gepackt werden müssen zusammenzustellen. Im einfachsten Fall schreibst du eben einfach alle Daten in ein Array und den Namen der Sicht in eine Variable.
                            • Wieder zurück in der index.php (da dieser Mechanismus wieder bei jedem Seitenaufruf identisch ist) wird view/$view.php eingebunden, also z.B. view/deluser_form.php oder view/deluser_success.php.
                            • Dort grifst du dann auf das Daten-Array zu.
                            • Da es auch in den Views, wenn du jede als eigene HTML-Seite konzipierst viele Redundanzen gibt, beschränkst du das HTML, das in den View-Dateien ausgegeben wird auf z.B. den Inhalt eines DIV-Containers der später fertigen Seite und sorgst auch in der index.php dafür, dass das Drumherum, das bei allen Seiten gleich ist überall ausgegeben wird. Entweder mit einem header- und einem footer-Template oder mit einem Haupttemplate, dass dann an der richtigen Stelle ein require "view/$view.php"; ausführt. (Mit Template meine ich hier eine HTML-Seite mit eingebettetem PHP-Code)

                            Das ganze ist ohne Objeke natürlich nicht wirklich doll zu lösen, aber ich hab so auch schon erfolgreich eine Webanwendung umgesetzt und das ist in jedem Fall ein Schritt in Richtung eleganteres Design.

                            Für den Fall, dass meine Lösung so ok ist: Ist die Verwenung von ID und isset auch sinnvoll? Oder kann man das noch "eleganter" lösen?
                            Die Bezeichnung "ID" ist nicht gut getroffen. Ich verwende bei sowas, wenn überhaupt, "sent". Anstatt "true" würde ich auch eher eine 1 nehmen, da hier ja die Zeichenkette "true" ankommt, anstatt der boolsche Wert true.

                            Wenn du allerings im action-Attribut des Forulars die Aktion eindeutig definierst, kannst du einfach anhand dieses Wertes prüfen, welches Formular da ankommt. Die Daten selbst musst du ja eh noch prüfen.

                            Was das einrücken angeht, so benutze ich inzwischen immer Tabs. Wichtig ist letztlich, dass du dich auf ein einheitliches Bild festlegst - das für dich übersichtlich ist und auf allgemein Üblichem aufbaut. Schau dir dazu z.B. mal die Coding Conventions/Guidelines von PEAR oder coWIKI oder ... an.

                            Basti

                            Kommentar


                            • #15
                              Zitat von Basti
                              Es ist Hacke wie Jose - allerdings würde ich mir die Klammern sparen.
                              Bis auf die angenehme Eigenschaft, dass echo mehrere Parameter aufnehmen kann.
                              Statt vielen, unnötigen Zeichenkettenverknüpfungen bei print('a'.'b'.'c'.'d'.'e'); benutze ich lieber echo 'a', 'b', 'c', 'd', 'e';

                              Andereseits kann man echo nicht wie print als Funktion "benutzen".
                              zB array_map('echo', $a); geht nicht , array_map('print', $a); dagegen schon.
                              edit: stimmt nicht mal. Auch mit print geht das nicht.

                              Kommentar

                              Lädt...
                              X