LX: Javascript verbessern

Beitrag lesen

Hallo, JSBETTERER!

Zuallererst: als Datenformat verwendest Du hier Dash-separierten Text, was insbesondere dann zum Problem werden kann, wenn eine der Daten selbst ein | enthalten könnte. Weiterhin schreibst Du eben diesen Text teilweise ohne weitere Prüfung in das innerHTML Deiner Seite, was wiederum ein Einfallstor für Cross-Site-Scripting darstellen könnte.

Es wäre zur Steigerng der Sicherheit empfehlenswert, 1. JSON als Format zu wählen (korrekter Parser vorausgesetzt) und 2. die Werte, die eingefügt werden sollen, mittels einer RegExp zu validieren.

Als nächstes steht beim Befüllen von auctionprice2 ein leeres var ohne Variable oder Deklaration, was normalerweise zu einem Fehler führen sollte.

Dann hast Du hier einen ziemlichen Spaghetticode (ohne Carbonara), der ziemlich wiederkehrende Aktionen (das Befüllen eines Nodes mit funktional aus den GET-Daten errechneten Informationen) untereinander schreibt. Das kann man durch eine Schleife über ein Objekt eleganter (und auch flexibler) handhaben:

// gather data in object "data"
var fields = {
   'auctionprice': function() { return (data.price || '?').replace(/<.*?>/,''); },
   'auctionprice2': ...
}, field;
for (field in fields) {
   if (!fields.hasOwnProperty(field)) { continue; }
   (document.getElementById(field) || {}).innerHTML = fieldsfield;
}

Zuletzt fällt mir auf, dass Du offenbar sehr viele Dinge als gegeben annimmst, was die Robustheit Deines Codes deutlich reduziert. Was, wenn ein Feld gelöscht wurde? Was, wenn ein Interval nicht gesetzt ist? Was, wenn das DOM noch nicht komplett befüllt ist oder getReq() keinen XMLHttpRequest zurückliefert?

Gruß, LX

--
RFC 1925, Satz 2: Egal, wie fest man schiebt, ganz gleich, wie hoch die Priorität ist, man kann die Lichtgeschwindigkeit nicht erhöhen.