Verbesserungsvorschläge

[suiko76]

Hallo Leutz,
ich hab in Windeseile und mit fast kaum Erfahrungen in PHP ein Script zusammengeschraubt, dass zwar soweit auch funktioniert, allerdings wohl kaum der ordentlichen PHP-Sprache entspricht. Ein Profi wird hier sicherlich die Hände über´m Kopf zusammen schlagen.. Naja es war halt dringend. Jetzt würd ich mich freuen, wenn sich jemand findet, der sich dem Script mal annehmen würde und es mal "aufräumt" bzw. ein, zwei Wünsche, die ich selbst nicht umsetzen konnte einbindet. Dazu aber später mehr.

Hier Infos zum Script selbst:

Auf einer html-Seite reiseberichte.html wird eine Kundennummer eingegeben, diese wird beim Absenden an die abfrage.php per Variable übergeben. Dort wird eine SQL-Verbindung aufgebaut und entsprechende Infos zur eingegebenen Kundennummer angezeigt.
Der Benutzer hat dann wiederrum die Möglichkeit Textfelder auszufüllen und die Daten zu speichern. Das passiert beim Absenden. Die Daten werden dann in eine separate DB gespeichert.Das wars.

Punkte die fehlen:

- Überprüfung ob Kd nur aus Zahlen besteht
- Wenn zur Kd und zum Wert "BTag" bereits ein Eintrag besteht, darf kein neuer erstellt werden
- Nach Absenden der eingetragenen Daten soll die Datei reisekosten.html wieder aufgerufen werden.

Hier der Link
http://www.schumacher.de/php/

[Boecki91]

Verlinke mal die Datei, so viel Code hier ist unübersichtlich, und ohne Hervorhebungen auch recht unleserlich.

Dann wäre es noch interessant zu wissen was das Script machen soll, momentan müsste man sich erst in den Code einarbeiten um überhaupt zu wissen "Das ist ein China-Sack-Reis anzeige- Code".

Dann würde ich dir empfehlen klare Variablen zu benutzen, was soll denn "$s1b"sein? Die ganzen 9 ersten Variablen sind wofür da? Warum ist s1 s1a s1b fast gleich?

Warum verwendest du mal ' und mal ", warum mal $sqlab4 =" und mal $sqlab5=" verwendet?
Und warum die Kommentare auf Deutsch, die Fehlermeldungen auf Englisch und die normale Ausgabe auf Deutsch ist vielleicht auch noch wichtig, hast du den Code zusammen kopiert? Auch das einrücken ist nicht einheitlich, teilweise passen die Fehlermeldungen nicht zum Code

Code: Alles auswählen

mssql_select_db($db1Name) or DIE("Table unavailable"); 

Dan zum Thema Sicherheit:
Prüfst du z.B: die Kundennummer auf bestimmte Kriterien, z.B. ob es eine Zahl ist

Verlinke also die Datei (KB:datei) und verwende einen einheitlichen Coding-Stil und beschreibe ausführlich was gemacht werden soll, da das recht viele DB-Sachen drin sind wäre die DB-Struktur eventuell noch wichtig.

[gn#36]

Du hast zuviel Code gepostet, was das Thema unnötig in die Länge zieht und unübersichtlich macht.
Es wäre besser die Datei bzw. den Code als .txt Datei abzuspeichern und dann hier einen Link zu posten - siehe auch KB:datei .

Bitte passe deinen Beitrag an, lies Dir den phpBB.de-Knigge und den Hinweis zur Fragestellung und Rechtschreibung durch und beachte diese zukünftig.

[suiko76]

Verlinke mal die Datei, so viel Code hier ist unübersichtlich, und ohne Hervorhebungen auch recht unleserlich.

Passiert

Dann wäre es noch interessant zu wissen was das Script machen soll, momentan müsste man sich erst in den Code einarbeiten um überhaupt zu wissen "Das ist ein China-Sack-Reis anzeige- Code".

Hab ich nun auch geschrieben

Dann würde ich dir empfehlen klare Variablen zu benutzen, was soll denn "$s1b"sein? Die ganzen 9 ersten Variablen sind wofür da? Warum ist s1 s1a s1b fast gleich?

Das WaWi-System verwendet in verschiedenen Tabellen für die selbe Saison
unterschiedliche Saison-Kennzahlen. D.h. das die Variable $s1, $s1a und $s1b
im Grunde Saison 81 bedeuten.

Warum verwendest du mal ' und mal ", warum mal $sqlab4 =" und mal $sqlab5=" verwendet?

$sqlab4 frägt was anderes ab als $sqlab5

Und warum die Kommentare auf Deutsch, die Fehlermeldungen auf Englisch und die normale Ausgabe auf Deutsch ist vielleicht auch noch wichtig, hast du den Code zusammen kopiert?

Zum Teil ja.. Die Sprache ist erst mal unwichtig..Das ändere ich noch

Auch das einrücken ist nicht einheitlich, teilweise passen die Fehlermeldungen nicht zum Code

Code: Alles auswählen

mssql_select_db($db1Name) or DIE("Table unavailable"); 

Dan zum Thema Sicherheit:
Prüfst du z.B: die Kundennummer auf bestimmte Kriterien, z.B. ob es eine Zahl ist

Das fehlt noch

Verlinke also die Datei (KB:datei) und verwende einen einheitlichen Coding-Stil und beschreibe ausführlich was gemacht werden soll, da das recht viele DB-Sachen drin sind wäre die DB-Struktur eventuell noch wichtig.

[Boecki91]

is_numeric()
is_int()

Beides mal auf php.net nachschlagen, was ich gerade noch gesehen habe ist:

strlen

[suiko76]

is_numeric()
is_int()

Beides mal auf php.net nachschlagen, was ich gerade noch gesehen habe ist:

strlen

Danke.. wobei ich dazu sagen muss, dass ich selbst nicht wirklich in der Lage bin, dass alles nachzuschlagen. Das würde ich normalerweise auch tun, nur fehlt mir u.a. die Zeit dazu. Ich würde mich echt riesig freuen, wenn mir jemand hier unter die Arme greifen könnte....
Hoffe, das ich echt nicht zuviel verlangt..

[Boecki91]

Die Funktionen sind eigendlich recht leicht nachzuschlagen

Code: Alles auswählen

if(!is_int($Zahl))
{
    die("Du sollst eine Zahl angeben")
}

Alle von mir vorgeschlagenen Funktionen liefern im Erfolgsfall true zurück, und veränder $Zahl nicht.

Ein wenig Zeit solltest du schon investieren, der Code von dir hat sicherlich schon viel Zeit gekostet, und die 5 Min lesen sind da nicht wirklich viel.

[suiko76]

Die Funktionen sind eigendlich recht leicht nachzuschlagen

Code: Alles auswählen

if(!is_int($Zahl))
{
    die("Du sollst eine Zahl angeben")
}

Alle von mir vorgeschlagenen Funktionen liefern im Erfolgsfall true zurück, und veränder $Zahl nicht.

Ein wenig Zeit solltest du schon investieren, der Code von dir hat sicherlich schon viel Zeit gekostet, und die 5 Min lesen sind da nicht wirklich viel.

Um genau zu sein: 1 1/2 Wochen

[suiko76]

Die Funktionen sind eigendlich recht leicht nachzuschlagen

Code: Alles auswählen

if(!is_int($Zahl))
{
    die("Du sollst eine Zahl angeben")
}

Alle von mir vorgeschlagenen Funktionen liefern im Erfolgsfall true zurück, und veränder $Zahl nicht.

Ein wenig Zeit solltest du schon investieren, der Code von dir hat sicherlich schon viel Zeit gekostet, und die 5 Min lesen sind da nicht wirklich viel.

Woran kann es liegen, dass das JS nicht ausgeführt wird? Wenn ich auf Check klicke, dann springt er sofort auf die php Seite.

Code: Alles auswählen

<script language="JavaScript">
function checkinput()
{
if(!is_int("kn"))
{
    die("Du sollst eine Zahl angeben")
}
}
   </script>
   </head>
   <body style="background-color: rgb(192,192,192)">
<center><img src="assets/images/autogen/a_logo_copy.gif" width="944" height="81" alt=""></center>
<b><font face="Arial" size="6"><center>Reiseberichte</font></b></center>
<br> <br> <br><b><font  color=#FF0033" face="Arial" size="5"><center>Under Construction</font></b></center>
<br><br> <br><br>  <br>  <br>
<center>  <form onSubmit="checkinput()"  action = "abfrage.php" method = "post">
<input type="text" name = "kn" size="5" maxlength="5"> <b>Kundennummer<p></b>
<input type = "submit" onClick="check();" value="Check">
<input type = "reset"></center>
</form>

[Boecki91]

Mein Code war kein JS, du solltest die Syntax nach 1,5 Wochen eigentlich erkennen, auch wenn ich ein ; am Ende vergessen habe.