Naps: Login Klasse/Meinung/Verbesserungsvorschläge

Beitrag lesen

HI,

  1. Deine Klasse sollte allein die Aufgabe haben, über die Korrektheit von angegebenen User-Authentifizierungsdaten zu entscheiden. Sie hat aber zusätzlich auch noch eine DB-Connection intern eingebaut. Sowas ist schlechte Abstraktion - was machst du, wenn andere Teile deines Codes auch auf eine Datenbank zugreifen wollen?

Das mit der DB-Connection werd ich in eine andere Klasse packen bzw. hab ich eigentlich schon eine die ich so verwende.

Was meinst du genau mit "sollte allein die Aufgabe haben, über die Korrektheit zu entscheiden" ?

  1. Dein Code ist außerdem schlecht wiederverwendbar, weil alle wichtigen internen Eigenschaften und Methoden private sind, anstelle protected zu sein. Nur mit letzteren werden sie auch vererbt. Private sollte man sehr zurückhaltend und mit Verstand einsetzen, denn es behindert Vererbung - und Vererbung ist eines der Grundbausteine von OOP.

ok...

  1. Vom Coding-Style her ist es ebenfalls unvorteilhaft, dass du deine public-Methoden nicht als solche gekennzeichnet hast. So wie das derzeit geschrieben ist, sieht das sehr nach einer Übernahme aus PHP 4 aus, selbst wenn das nicht stimmen sollte.

Ja was das betrifft werd ich mir das nochmal anschauen, das hab ich glaub ich falsch verstanden.

  1. session_unset() ist uralt-ekliger PHP 4.0-Session-Krams und gehört, gemeinsam mit session_register(), entsorgt. Ich bin mir auch nicht sicher, dass du weißt, was session_destroy() genau macht. Es ist ebenfalls überflüssig, die relevanten Lösch-Vorgänge löst man heutzutage mit entsprechenden Aktionen auf $_SESSION aus.

Soll ich da einfach jeden Wert von $_SESSION löschen?

  1. Dass dein Datenbankzugriff Prepared Statements benutzt, macht die Sache nicht wirklich einfach zu handhaben, oder? Performance gewinnst du damit sicherlich nicht, und das Rudel lokaler Variablen, die damit erzeugt werden, sind für meine Augen alles andere als übersichtlich. Du vermeindest damit natürlich vermeintlich, dass böse Userdaten dir dein SQL injizieren - allerdings setzt du trotzdem deine Querys dynamisch zusammen, d.h. wenn ein Angreifer dir in die Variable $this->Config['TablePrefix'] was einschmuggelt, bist du genauso schlecht dran, wie mit Escaping und der traditionellen Methode.

ok, hab gedacht dass es die beste Lösung wäre, werd ich noch überarbeiten.

  1. Der Zugriff auf superglobale Variablen innerhalb der Klasse ist hinsichtlich Wiederverwendbarkeit und Kapselung ebenfalls kritisch zu betrachten. Die superglobalen Variablen sind für manchen prozeduralen Code eine gute Erfindung gewesen, letztendlich aber sind es ganz normale globale Variablen - und globale Variable sind böse und sollten unbedingt vermieden werden. Auch wenn sie $_SESSION, $_COOKIE oder $_GET heißen.

Was meinst du hier genau?

Nochmal danke für die Ausführlichen Vorschläge,
MfG Naps