Zastąpienie ifów w managedBeanie

0

Witam. Mam takiego JSFowego beana, w którym mam metodę tworzącą użytkownika:

    public String createNewAccount() {
        //pobranie parametrów sesji
        String userRole = requestParameterMap.get("userRole").toString();
        String active = requestParameterMap.get("active").toString();
        Boolean act = Boolean.parseBoolean(active);
        //ustawienie pobranych parametrów użytkownikowi
        user.setRole(userRole);
        user.setActive(act);

        String result = userService.create(user); //wywołanie metody tworzącej nowe konto użytkownika

        //sprawdzenie czy podane dane nie istnieją już w bazie
        if (result.equals("emailExist")) {
            sendErrorMessageToUser("Użytkownik z podanym adresem email istnieje w bazie");
            return null;
        } else if (result.equals("personalIdExist")) {
            sendErrorMessageToUser("Użytkownik z podanym numerem pesel istnieje w bazie");
            return null;
        } else if (result.equals("phoneExist")) {
            sendErrorMessageToUser("Użytkownik z podanym numerem telefonu istnieje w bazie");
            return null;
        } else if (result.equals("ok")) {
            return "newAccountCreated";
        } else {
            sendErrorMessageToUser("Błąd dodawania użytkownika do bazy");
            return null;
        }
    }

Wywoływana jest w niej metoda

userService.create(user);

która zwraca różne stringi, jeśli np. ktoś z podanym emailem, telefonem lub peselem istnieje w bazie, jeśli nie ma to zwraca "ok".

I tutaj moje pytanie bo ktoś mi kiedyś powiedział, że takie coś jest nieestetyczne, chodziło mianowicie o te Stringi oraz ify, że taki styl programowania jest na poziomie początkującego. Niestety nie miałem jak już dopytać tej osoby w jaki inny sposób to zrealizować. I tutaj moje pytanie do kogoś kto w tym siedzi. W jaki inny sposób można to zrealizować? Czym zastąpić te zwracane Stringi oraz te kilka ifów? Mi osobiście nic do głowy nie przychodzi.

1

HashMap?

HashMap<String,String> messages = new HashMap<String,String>();
messages.put("emailExist","Użytkownik z podanym adresem email istnieje w bazie");
...
0

Nie wiem bardzo jak to miałoby zlikwidować mi ify i stringi?

0

@olek1 jak to jak? Z mapy możesz w jednej linijce wyciągnąć sobie wiadomość na podstawie stringa, bez żadnych ifów. Pytanie czy w ogóle takiej logiki nie powinieneś wrzucic do walidatora...

1
sendErrorMessageToUser(messages.get(result));
0
Shalom napisał(a):

@olek1 jak to jak? Z mapy możesz w jednej linijce wyciągnąć sobie wiadomość na podstawie stringa, bez żadnych ifów. Pytanie czy w ogóle takiej logiki nie powinieneś wrzucic do walidatora...

No chyba nie. W walidatorze sprawdzam zgodność wprowadzonych danych z wzorcem. Czyli czy telefon składa się z samych cyfr itp. A to czy podane dane istnieją w bazie sprawdzam w warstwie serwisowej. Błędne jest takie podejście? W takim razie musiałbym do walidatora wstrzyknąć zależność żeby przeszukać bazę.
Kiedyś chyba nawet opisywałem to na forum to ktoś mi właśnie napisał, że w walidatorze powinno się tylko sprawdzać poprawność wprowadzanych danych, a weryfikację czy taki wpis już istnieje w warstwie serwisowej.

Co do tej hashMapy to nie wiem czy do końca to rozwiąże problem, bo co w przypadku gdy dostaniemy "ok"?

1

To nawet ja ci odpisywalem ;) Fakt, w takiej sytuacji walidator się nie zda. Ale ciekawi mnie po co w ogóle chcesz wypisywać taki błąd? Ogólnie to dość zła praktyka bo udostępniasz użytkownikowi informacje których nie powinien mieć. Tak samo jak ktoś sie loguje to nie wolno pisać "podałeś złe hasło" albo "podałes zły login" bo w ten sposób zdradzasz pewne informacje.

I nie rozumiem czemu user service zwraca jakiegoś stringa. Na oko to powinien zwrócić jakiś obiekt albo rzucić wyjątek. Wtedy nie miałbyś kłopotu z tymi swoimi ifami i mapą ;)

1

@olek1 napisał

Co do tej hashMapy to nie wiem czy do końca to rozwiąże problem, bo co w przypadku gdy dostaniemy "ok"?

String msg = messages.get(result);
if(msg!=null)
{
     sendErrorMessageToUser(msg);
}
2

Zamiast String zwracaj z serwisu obiekt w rodzaju:

class CreateUserResult{
    Set<Error> errors;
    OperationStatus status;
}

Następnie zamiast serii ifów:

if(cur.status==EXIST){
    sendMessage(cur.errors.get(0).getMessage());
}

Klasa Error powinna zawierać flagę odwołującą się do properties, czyli coś w rodzaju:

String key = "errors.emailExist";
String getMessage(){   
    return Resources.getErrorDictionary().messageFor(this.key);
}

Dzięki temu można jeszcze dodatkowo ustawiać wersje językowe komunikatów ;)

1 użytkowników online, w tym zalogowanych: 0, gości: 1