Czy takie używanie wyjątków ma sens?

Question

Mam następujące pytanie: Czy takie użycie wyjątków jest poprawne, chodzi mi bardziej o fakt tego czy jest to zgodne z dobrymi praktykami? W poniższym przykładzie używam ich do walidacji pól formularza, ale wiem też że mógłbym to rozwiązać w inny sposób.

switch (true) {
      case $this->isEmpty([$uid, $pwd]):
        throw new FormException('Pozostawiono puste pola.');
        break;
      case !$this->userExist($uid);
        throw new UserException('Użytkownik nie został znaleziony.');
        break;
      case !$this->checkPwd($uid, $pwd):
        throw new UserException('Podano błędne hasło');
        break;
    }

Answer 1

Kilka rzeczy ode mnie. 

Rzucanie wyjątków w switchu nie jest błędem. Break jest tam nie potrzeby. Pomijam również kwestię, że wygląda to nieczytelnie i drabinka ifów załatwiłaby sprawę. 

Kolejny temat to pomysł korzystania z wyjątków. Moim zdaniem problem walidacji wcale nie wymaga rzucania wyjątków w ramach poinformowania czy dane są poprawne. Oczywiście mówię tutaj o kodzie samej walidacji, która jest de facto czymś całkowicie odrębnym od miejsca, w którym z niej korzystamy.

PS. Nazwij proszę tą metodę po prostu checkPassword.

Answer 2

rzucanie wyjątków w switch/case nie jest poprwane, tzn stosować tak możesz ale dobrą praktyką to nie jest, bo switch/case nie służy do obsługi czegoś takiego.
Swoją drogą jak już masz 2 błędy które, wyrzucają tę samą klasę, można by to sformalizować w jedną rzecz zależną od zmiennych

Comments

rzucanie wyjątków w switch/case nie jest poprwane, tzn stosować tak możesz ale dobrą praktyką to nie jest, bo switch/case nie służy do obsługi czegoś takiego.

Możesz podać źródło, które określa to jako złą praktykę? Pytam z ciekawości, bo switch jest instrukcją sterującą, więc dlaczego by za jej pomocą nie wykryć przypadku do rzucenia wyjątku? 

Answer 3

Według mnie samo użycie switcha jest spoko, jednak miałbym uwagę do klas wyjątków. Mianowicie najlepszą praktyką jest dostarczanie dedykowanego wyjątku dla każdego use case'a, tj.:

throw UserException::notFound();

throw UserException::invalidCredentials();

// lub

throw new UserNotFoundException();

throw new InvalidUserCredentialsException();

W ten sposób nie musisz ustawiać komunikatu, ponieważ każdy wyjątek ma taki komunikat predefiniowany.

Btw po instrukcji throw nie musisz robić break :)

Answer 4

Według mnie powinieneś tutaj użyć walidatora. Rzucanie wyjątkami przerywa działanie programu. To raczej nie jest wskazane w przypadku niepowodzenia logowania. Chyba że zapewniasz obsługę takich zdarzeń. Mimo wszystko to i tak jest zła praktyka i wygląda jak niezły work-around, bo wyjątek oznacza sytuację wyjątkową, w zasadzie niespodziewaną w działaniu programu, a nie w zachowaniu użytkownika. Zwykle programy są bardziej deterministyczne niż zachowanie użytkowników portalu ;)