ComputerBase
Aktuelles

PHP Kann man meinen Code kürzen?

phoenix430

phoenix430

Lt. Commander
Registriert
Juni 2008
Beiträge
1.446
Hallo Leute,

ich bin totaler PHP Anfänger und habe nun mein erstes mir selbst gesetztes Prjekt fertig gestellt, so das alles funktioniert. Nun habe ich aber Seitenweise Quellcode, so das ich schnell den Überblick verliere. Meine SQL Abfrage habe ich nun ausgelagert
Datenbank.php
PHP: 
<?php
$mysqlhost="localhost"; // MySQL Host angeben
$mysqluser="root"; // MySQL User angeben
$mysqlpw=""; // Passwort angeben
$mysqldb="mediathek"; // Datenbank angeben

$db=mysql_connect($mysqlhost,$mysqluser,$mysqlpw) or die ("Verbindungsversuch fehlgeschlagen");
mysql_select_db($mysqldb) or die ("Konnte die Datenbank nicht finden");
?>
aufrufen tuhe ich es dann hiermit
PHP: 
require("DatenbankSQL.php");
Ist das so weit richtig?

Meine nächste Frage ist, ob ich diesen Code auch irgendwie kürzen kann?
PHP: 
// Abfrage zusammen bauen

$abfrage="";
if ($titel <> ""){
    $suchworte = explode(" ",$titel);
    $count = count($suchworte);
    $abfrage.="(";
    for($i=0;$i <$count;$i++){
        $abfrage.="Titel LIKE '%".$suchworte[$i]."%'";
        if ($i<($count-1)){
            $abfrage.= " AND ";
        }
    }
    $abfrage.= " ) ";
}
if ($beschreibung <> ""){
    $suchworte = explode(" ",$beschreibung);
    $count = count($suchworte);
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
         $abfrage.= " (";
    }
    for($i=0;$i <$count;$i++){
        $abfrage.="Bemerkungen LIKE '%".$suchworte[$i]."%'";
        if ($i<($count-1)){
            $abfrage.= " AND ";
        }
    }
    $abfrage.= " ) ";
}
if ($begleitmaterial <> ""){
    $suchworte = explode(" ",$begleitmaterial);
    $count = count($suchworte);
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    for($i=0;$i <$count;$i++){
        $abfrage.="Begleitmaterial LIKE '%".$suchworte[$i]."%'";
        if ($i<($count-1)){
            $abfrage.= " AND ";
        }
    }
    $abfrage.= " ) ";
}
//  ID Liste

if ($typ <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_typ = ".$typ;
    $abfrage.= " ) ";
}
if ($art <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_art = ".$art."";
    $abfrage.= " ) ";
}

if ($fachbereich <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_fachbereich = ".$fachbereich."";
    $abfrage.= " )";
}

if ($schwierigkeit <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_schwierigkeit = ".$schwierigkeit."";
    $abfrage.= " )";
}

if ($besetzung <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_besetzung = ".$besetzung."";
    $abfrage.= " )";
}

if ($genre <> 0){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.="KAT_genre = ".$genre."";
    $abfrage.= " )";
}
//  Liste

if (($herausgeber <> "") AND ($herausgeber <> "Bitte wählen")){
    if ($abfrage <> "") {
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.='herausgeber = "'.$herausgeber.'"';
    $abfrage.= " )";
}

if (($ort <> "") AND ($ort <> "Bitte wählen")){
    if ($abfrage <> "") {
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.='ort = "'.$ort.'"';
    $abfrage.= " )";
}

if (($standort <> "") AND ($standort <> "Bitte wählen")){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.='standort = "'.$standort.'"';
    $abfrage.= " )";
}

if (($komponist <> "") AND ($komponist <> "Bitte wählen")){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.='komponist = "'.$komponist.'"';
    $abfrage.= " )";
}

if (($verlag <> "") AND ($verlag <> "Bitte wählen")){
    if ($abfrage <> ""){
        $abfrage.= " AND (";
    }
    else {
        $abfrage.= " (";
    }
    $abfrage.='abfrage = "'.$abfrage.'"';
    $abfrage.= " )";
}
 
Bau doch Funktionen:

Code: 
function add_arguments1($pvar, $pabfrage)
{
if (($pvar <> "") AND ($pvar <> "Bitte wählen")){
    if ($pabfrage<> ""){
        $pabfrage.= " AND (";
    }
    else {
        $pabfrage.= " (";
    }
    $pabfrage.='pabfrage= "'.$pvar .'"';
    $pabfrage.= " )";
}  

return $pabfrage;
}

$abfrage = add_arguments1($verlag, $abfrage);
...


Übrigens ist in der drittletzten Zeile ein Fehler drin.
 
Stefan- schrieb:
Bau doch Funktionen:

Code: 
function add_arguments1($pvar, $pabfrage)
{
if (($pvar <> "") AND ($pvar <> "Bitte wählen")){
    if ($pabfrage<> ""){
        $pabfrage.= " AND (";
    }
    else {
        $pabfrage.= " (";
    }
    $pabfrage.='pabfrage= "'.$pvar .'"';
    $pabfrage.= " )";
}  

return $pabfrage;
}

$abfrage = add_arguments1($verlag, $abfrage);
...


Übrigens ist in der drittletzten Zeile ein Fehler drin.
Zum Vergrößern anklicken....

Danke das werde ich mal ausprobieren.
Danke für den Hinweis werde ich mal überprüfen.
 
dann kannst noch weiter kürzen.

zb

Code: 
    if ($pabfrage<> ""){
        $pabfrage.= " AND (";
    }
    else {
        $pabfrage.= " (";
    }

zu

PHP: 
$pabfrage = ($pabfrage != "") ? " AND (" : " (";

und so wie ich den code verstehe baust du da ungefilter mit usereingaben deinen query zusammen!
gefährlich für sql injections
 
Mit den ?: If Abfragen würde ich sparsam umgehen.
Ist zwar durchaus manchmal angenehm herzunehmen und verkürzt auch den Code, macht ihn aber nicht zwingend leserlicher.

Generell ist es mit PHP mMn schwierig, "sauberen" Code zu produzieren, weil PHP dazu viel zu tolerant ist. Wenn man nicht selbst konsequent genug ist, erzeugt man sehr schnell "Chaos".

Wichtig ist also in erster Linie, dass man sich seinen eigenen Stil zu legt und diesen auch konsequent verfolgt. Woran man sich dabei orientiert, sollte jeder für sich selbst entscheiden.

Ein Beispiel hierbei wäre dein Ungleich Operator, ich tendiere hier zum Beispiel eher zu != anstatt <>
Generell finde ich es jedoch sauberer, nicht auf <> "" zu überprüfen, sondern die Funktion empty zu benutzen (bitte wichtig: Dokumentation dazu lesen, um Fehler zu vermeiden)

Zudem hab ich mir auch angewöhnt, typsicher zu überprüfen, damit kann man einige Fehler vermeiden, die sich oft sehr schwer finden lassen, wenn man diese "Tücke" von PHP nicht kennt.

Was mir sonst noch auffällt: require (und include usw) ruft man eig nicht über () auf, sonder nur mit require 'file.php';
Aber wieder ein typischer Fall, PHP erlaubt beides.

Bevor ich meine Stilpredigt beende, in die ich mich gerade reingeredet habe, hab ich es mir auch angewöhnt, in PHP die selben Quotes zu verwenden (wobei ich zu Single Quotes tendiere, mit Ausnahme von SQL Statements generieren)

Generieren ist dann eigentlich auch schon mein eigentliches Stichwort, um den Code zu verkürzen.

Momentan überprüfst du alles immer einzeln und setzt so dein Query zusammen. Wie stefan das schon richtig bemerkt hat, ist das ein schöner Fall, um Funktionen zu benutzen bzw. kennen zu lernen. Ich will jetzt auch gar nicht weiter ins Detail gehen, aber du wirst recht bald, je nach Lernfortschritt merken, dass man sich sehr sehr viel ersparen kann, wenn man Code abstrahiert. Funktionen sind hierzu ein erster Schritt, aber mit Objekt Orientierter Programmierung (OOP) und guter Logik kann man das noch weiter führen.

Aber wie gesagt, das sind alles Erfahrungswerte, und man sollte nicht die Welt von heute auf morgen einreißen wollen, sondern das ganze langsam angehen und sich immer weiter entwickeln und natürlich informieren.

Aber wenn du mal Funktionen und in weiterer Folge OOP beherrscht, dann wirst du vllt auch irgendwann mal soweit sein, dass du ein SQL Query gar nicht mehr zusammen bauen musst, sondern zB Datenbankunabhängig on demand generieren lässt.

Naja, irgendwie hab ich das Gefühl, dass ich jetzt viel gelabert hab, von Stil und Abstraktion usw, und dir eig. nicht viel geholfen hab, aber vielleicht kannst ja doch etwas für dich mitnehmen :D


so long
 
Bei den ganzen Bedingungen finde ich es ratsam sie in ein Array zu packen und anschließend mit implode() eine Zeichenkette zu bilden. Dann fallen schonmal so einige Konstrukte weg.
 
zu der typensicheren überprüfung ist im übrigen zu sagen, dass das ganze durchaus auch nen performance vorteil mit sich bringt! das ist nich zu verachten und beugt vorallem bei konsequenter anwendung auch sicherheitslücken vor ;)
 
Sry für die späte Antwort, aber ich hatte keine Zeit gefunden. Danke für eure Antworten.
Ich habe es jetzt so probiert:
PHP: 
function add_arguments1($pvar,$argtitel,$fkt) {
  global $abfrage;
   if (($pvar <> "") AND ($pvar <> "Bitte wählen")) {
     if($abfrage<>""){
         $abfrage.=" AND ";
     }
     if ($fkt==1){  // $fkt =1: Textfeld -> alle anderen Werte ID-Feld

      $pabfrage.= " ( ";
      $suchworte = explode(" ",$pvar);
      $count = count($suchworte);
      for($i=0;$i <$count;$i++){
         $pabfrage.= $argtitel." LIKE '%".$suchworte[$i]."%' ";
          if ($i<($count-1)) {
             $pabfrage.= " AND ";
          }
      }
     } else {
       ($pvar <> 0) {  // ID-Feld
           if ($pabfrage <> ""){
               $abfrage.= " AND (";
           }
           else {
               $pabfrage.= " (";
           }
          $pabfrage
       }
     }
      $pabfrage.= " ) AND ";
      return $pabfrage;
   }
   return '';
}
Irgendwas stimmt aber noch nicht? Kann mir vlt jemand sagen wo bei mir der Fehler ist?
 
Bei deinem Code fehlt ein if am Anfang der folgenden Zeile:
PHP: 
       ($pvar <> 0) {  // ID-Feld
Aber selbst, wenn du das korrigierst, gibt es noch ein Problem. Einen Teil der SQL-Abfrage hängst du direkt an die globale Variable $abfrage an (ein sogenannter Neben- oder Seiteneffekt deiner Funktion) und der andere Teil der SQL-Anfrage ist der Rückgabewert deiner Funktion. Diese beiden Möglichkeiten sollte man möglichst nicht miteinander vermischen, weil sie zu schwer lesbarem und damit fehleranfälligen Code führen. Ich persönlich würde immer die Methode mit dem Rückgabewert bevorzugen.

Außerdem kann man an deinem Code die folgenden Vereinfachungen durchführen:
  1. Weil alle Bedingungen mit AND verknüpft sind, kann man die Klammern komplett weglassen.
  2. Man kann vor jede einzelne Bedingung ein AND setzen und erst, wenn man die ganze Bedingung benutzt, entfernt man dort das erste AND wieder (im Beispiel durch den Aufruf von substr()).
  3. Ich würde für Textfelder und ID-Felder jeweils eine eigene Funktion benutzen.
Außerdem sollte man den Code unbedingt gegen SQL-Injection absichern. Dann erhält man z. B. so etwas:
PHP: 
function erzeuge_bedingung_text($name, $text) {
	$bedingung = "";
	if (is_string($text) && $text !== "" && $text !== "Bitte wählen") {
		$suchworte = explode(" ", $text);
		for ($i = 0; $i < count($suchworte); $i++) {
			$bedingung .= " AND " . $name . " LIKE '%" . mysql_real_escape_string($suchworte[$i]) . "%'";
		}
	}
	return $bedingung;
}

function erzeuge_bedingung_zahl($name, $zahl) {
	$bedingung = "";
	if (is_int($zahl) && $zahl > 0) {
		$bedingung .= " AND " . $name . " = " . $zahl;
	}
	return $bedingung;
}

$bedingung = "";
$bedingung .= erzeuge_bedingung_text("Titel", "abc def");
$bedingung .= erzeuge_bedingung_text("Beschreibung", "");
$bedingung .= erzeuge_bedingung_zahl("KAT_typ", 2);
$bedingung .= erzeuge_bedingung_zahl("KAT_art", 0);

$abfrage = "SELECT * FROM tabelle";
if ($bedingung !== "") {
	$abfrage .= " WHERE" . substr($bedingung, 4);
}
Am Ende hat die Variable $abfrage dann den folgenden Wert:
PHP: 
SELECT * FROM tabelle WHERE Titel LIKE '%abc%' AND Titel LIKE '%def%' AND KAT_typ = 2
 
Danke für deine Antwort. Das mit dem SQL injection würde ich gerne nach dem kürzen meines Quellcodes in Angriff nehmen.
Die Problematik ist, dass ich eine Abfrage zusammen baue, je nachdem was man vorher hier ausgewählt hat:
PHP: 
<?php

require("DatenbankSQL.php");

function Liste ($abfrage){
   $ergebnis = mysql_query($abfrage);
   $anzeige = array();
while($row = mysql_fetch_object($ergebnis))
   {
   $anzeige[] = $row->bezeichnung;
   }
foreach($anzeige as $value){
   echo '<option value="'.$value.'">'.$value.'</option>';
   }
}

function IdListe ($abfrage){
   $ergebnis = mysql_query($abfrage);
   $anzeige = array();
while($row = mysql_fetch_object($ergebnis))
   {
   echo '<option value="'.$row->id.'">'.$row->bezeichnung.'</option>';
   }
}

echo '
<form action="ausgabe.php" method="post">
Typ: <select name="typ" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT bezeichnung, id FROM kat_typ ");
echo '</select>';
echo '
Titel enthält <input type="text" name="titel" size="40"/>';
echo '
Herausgeber enthält <select name="herausgeber" id="dropdown">
<option value="Bitte wählen" selected="selected">Bitte wählen</option>';
Liste ("SELECT Herausgeber AS bezeichnung FROM titel WHERE (Herausgeber IS NOT NULL AND Herausgeber <>'') GROUP BY Herausgeber");
echo '</select>';
echo '
Ort enthält <select name="ort" id="dropdown">
<option value="Bitte wählen" selected="selected">Bitte wählen</option>';
Liste ("SELECT Ort AS bezeichnung FROM titel WHERE (Ort IS NOT NULL AND Ort <>'') GROUP BY Ort");
echo '</select>';
echo '
Standort enthält <select name="standort" id="dropdown">
<option value="Bitte wählen" selected="selected">Bitte wählen</option>';
Liste ("SELECT Standort AS bezeichnung FROM titel WHERE Standort IS NOT NULL GROUP BY Standort");
echo '</select>';
echo '
Beschreibung enthält <input type="text" name="beschreibung" size="40"/> ';
echo '
Begleitmaterial enthält <input type="text" name="begleitmaterial" size="40"/> ';
echo '
Art <select name="art" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT Bezeichnung AS bezeichnung,ID AS id FROM kat_art");
echo '</select>';
echo '
Fachbereich <select name="fachbereich" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT Bezeichnung AS bezeichnung,ID AS id FROM kat_fachbereich");
echo '</select> ';
echo '
Schwierigkeit <select name="schwierigkeit" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT Bezeichnung AS bezeichnung,ID AS id FROM kat_schwierigkeit");
echo '</select> ';
echo '
Komponist enthält <select name="komponist" id="dropdown">
<option value="Bitte wählen" selected="selected">Bitte wählen</option>';
Liste ("SELECT Komponist AS bezeichnung FROM titel WHERE (Komponist IS NOT NULL AND Komponist <> '')GROUP BY Komponist");
echo '</select> ';
echo '
Verlag enthält <select name="verlag" id="dropdown">
<option value="Bitte wählen" selected="selected">Bitte wählen</option>';
Liste ("SELECT Verlag AS bezeichnung FROM titel WHERE (Verlag IS NOT NULL AND Verlag <> '')GROUP BY Verlag");
echo '</select> ';
echo '
Besetzung <select name="besetzung" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT Bezeichnung AS bezeichnung,ID AS id FROM kat_besetzung");
echo '</select> ';
echo '
Genre <select name="genre" id="dropdown">
<option value="0" selected="selected">Bitte wählen</option>';
IdListe ("SELECT Bezeichnung AS bezeichnung,ID AS id FROM kat_genre");
echo '</select>
<input type="submit" value="Suche starten" name="abschicken">
</form>';
?>
Die Abfrage
PHP: 
SELECT * FROM tabelle WHERE Titel LIKE '%abc%' AND Titel LIKE '%def%' AND KAT_typ = 2
kann ich glaube ich so nicht nutzen, da ich am Ende meines Formulares folgendes benutze:
PHP: 
// Ausgabe
    $auswertung = "SELECT * FROM titel WHERE ";
    $auswertung.= $abfrage;

$ergebnis = mysql_query($auswertung);
while($row = mysql_fetch_object($ergebnis)){

error_reporting(-1);
    // meine erste Abfrage hier wird die Auswahl des users angezeigt
    echo '<tr>'."\r\n";
    echo '<td><a href = "weiterleiten.php?id='.$row->ID.'" target ="blank"/>'.$row->Titel.'</a></td>'."\r\n";
    echo '<td>'.$row->Komponist.'</td>'."\r\n";
    echo '<td>'.$row->Herausgeber.'</td>'."\r\n";
    echo '<td>'.$row->Verlag.'</td>'."\r\n";
    $id=$row->ID;
    // hier soll eine zweite Abfrage erfolgen, die dem user sagt ob ein Titel verfügbar ist oder nicht und in der Spalte ausgeliehen angezeigt
    $auswertung2 =  "SELECT * FROM ausgeliehen WHERE Titelnr = $row->ID AND ISNULL(rueckgabedatum)";
    $ergebnis2 = mysql_query($auswertung2);
    if(mysql_num_rows($ergebnis2)<>0 ){
       echo '<td>Titel ist ausgeliehen</td>'."\r\n";
       echo '</tr>';
    }
    else{
       echo '<td>Titel ist verfügbar</td>'."\r\n";
       echo '</tr>';
    };
 }
    echo '</table>';

Ich habe das mit dem if nun geändert. Kann ich denn Prinzipiell meinen Code so stehen lassen, oder müsste ich den trotzdem komplett ändern?
 
Das Zusammensetzen der Abfrage war nur als Beispiel gedacht. Du musst natürlich in deinem Programm die Bedingungen benutzen, nach denen du suchen möchtest. Also ungefähr so:
PHP: 
$abfrage = "";
$abfrage .= erzeuge_bedingung_zahl("KAT_typ", (int)$_POST["typ"]);
$abfrage .= erzeuge_bedingung_text("Titel", $_POST["titel"]);
// Hier auch noch die anderen Bedingungen hinzufügen...

$auswertung = "SELECT * FROM titel";
if ($abfrage !== "") {
    $auswertung .= " WHERE" . substr($abfrage, 4);
}
 
Du musst dich einloggen oder registrieren, um hier zu antworten.

Ähnliche Themen

H
Python Anmeldungscode optimieren (Kurzer Code)
Antworten
7
Aufrufe
1.888
Bitopium
Bitopium
Harsiesis
Arduino - Unterstützung bei Code
Antworten
2
Aufrufe
1.967
Harsiesis
Harsiesis
Z
C++ Code Optimierung.
Antworten
9
Aufrufe
2.381
ZuseZ3
Z
Ü
PHP PHP preg_replace - Code kürzen
Antworten
6
Aufrufe
1.502
Daaron
D
Endless Storm
Java leidiges Thema - static und non static, Frage zum Code
Antworten
7
Aufrufe
1.793
Endless Storm
Endless Storm
Zurück
Oben

Willkommen auf ComputerBase!

Mit Werbung weiterlesen

Besuche ComputerBase wie gewohnt mit Werbung und Tracking. Die Zustimmung ist jederzeit widerrufbar.

Details im Privacy Center und in der Liste unserer Partner. Ein Widerruf ist möglich in der Datenschutzerklärung.

… oder ComputerBase Pro bestellen

Nutze ComputerBase ohne Werbebanner, Video-Ads und Werbetracking schon für 4 €/Monat oder 36 €/Jahr.

Mehr zu ComputerBase Pro

Bereits Pro-Nutzer? Hier anmelden.

Tracking: Wir und unsere 177 Partner verarbeiten personenbezogene Daten, indem wir mit auf Ihrem Gerät gespeicherten Informationen (z. B. eindeutige Kennungen in Cookies) ein Nutzungsprofil erstellen, um z. B. Anzeigen zu personalisieren. Verarbeitungszwecke: Genaue Standortdaten und Abfrage von Geräteeigenschaften zur Identifikation, Informationen auf einem Gerät speichern und/oder abrufen, Personalisierte Anzeigen und Inhalte, Anzeigen- und Inhaltsmessungen, Erkenntnisse über Zielgruppen und Produktentwicklungen.

Impressum Kontakt Datenschutz