Aufteilung der Funktion, MVC

[Rolf B]

Hallo vapita,

du baust da schon hübsch sortiert was zusammen. Was ich ändern würde:

getQuery

dazu hatte ich schon mal was geschrieben. Diese Methode ist so, wie sie ist, Quatsch. Wenn Du schon Exceptions wirfst, dann lass sie auch fliegen und fang sie nicht gleich wieder ein. Was Du mit $queryException machst, ist mir schleierhaft: mit define festgelegte Konstanten tragen kein $, eine globale Variable müsste mit global deklariert werden, und statische Konstanten in der Klasse sollte ein Request:: oder self:: Präfix tragen. Soweit ich das sehe, läufst Du im Errorhandling auf eine undefinierte Variable. Das gleiche gilt für die Fehlerwerte aus User::validate.

register

Was befindet sich, wenn Du $this->user->validate aufrufst, im user Property des UserControllers? Wird da von getUser was hineingelegt? Aber welcher - wenn Du registrierst, gibt es ja keinen angemeldeten User. Dies scheint mir falsch konstruiert. Entweder sollte validate eine statische Methode sein, die Du mit User::validate(...) aufrufst, oder Du änderst das Vorgehen so, dass Du einfach erstmal den User konstruierst und darauf dann validate als Instanzmethode rufst. Hier wäre übrigens eine Stelle, wo man durchaus try/catch verwenden kann.

try {
   ...
   $user = new User($userData);
   $user->validate($userRepository);
   $this->getEntityManager()->persist($user);
   $this->flash->add(200);
   $this->redirect('302', 'user/login');
}
catch (Exception $validationException) {
   this->flash->add($validationException->getMessage(), 'danger');
}

Über Entitymanager und Repository wollte ich gerade auch schon anfangen, aber aber klugerweise mal gegoogelt. Du verwendest Symfony und Doctrine - das ist also so vorgegeben für Dich. Jetzt verstehe ich auch, warum Du fleißig set-Methoden verwendest. Das muss so, sonst bekommt Doctrine nicht mit, dass sich Propertywerte ändern.

class Password

Das sieht nach einer statischen Klasse aus. Eigentlich brauchst Du davon keine Instanz. Mach die Methoden static und ruf dann bspw. Password::hash(...) auf.

Rolf