@Shalom pomysł z walidatorami naprawdę dobry, dzięki. Napisałem coś takiego:
Interfejs dla walidatorów
Kopiuj
public interface UserDataValidator {
void validateCreateUser(User user) throws ValidationException;
void validateUpdateUser(User user) throws ValidationException;
}
ma on 3 implementacje PhoneValidator, EmailValidator, PersonalIdValidator, wrzucam jedną z nich poglądowo:
Kopiuj
public class EmailValidator implements UserDataValidator{
@PersistenceContext
private EntityManager EntityManager;
@Override
public void validateCreateUser(User user) throws InvalidEmailException{
if ((user.getEmail() != null) && (findEmailExist(user.getEmail()))) {
throw new InvalidEmailException();
}
}
@Override
public void validateUpdateUser(User user) throws InvalidEmailException {
if ((user.getEmail() != null) && (findEmailExist(user.getEmail(), user.getId()))) {
throw new InvalidEmailException();
}
}
//tutaj metody findEmailExist w 2 wariantach
}
Dodatkowo zrobiłem te wyjątki i tutaj mam pewne wątpliwości czy jest to dobre rozwiązanie:
Klasa abstrakcyjna wyjątków:
Kopiuj
public abstract class ValidationException extends Exception{
public ValidationException() {
}
public ValidationException(String message) {
super(message);
}
}
Również ma 3 implementacje InvalidPhoneException, InvalidPersonalIdException oraz PersonalIdException. Jedna poglądowa klasa:
Kopiuj
public class InvalidEmailException extends ValidationException{
public InvalidEmailException() {
}
public InvalidEmailException(String message) {
super(message);
}
}
Wtedy mój serwis wygląda tak:
Kopiuj
private void initValidator() {
userDataValidatorList = new ArrayList<>();
userDataValidatorList.add(new EmailValidator());
userDataValidatorList.add(new PersonalIdValidator());
userDataValidatorList.add(new PhoneValidator());
}
@Override
public UserOperationState create(User user) {
for(UserDataValidator validator : userDataValidatorList ) {
try {
validator.validateCreateUser(user);
} catch (InvalidEmailException ex) {
return UserOperationState.EMAIL_EXIST;
} catch (InvalidPersonalIdException ex) {
return UserOperationState.PERSONALID_EXIST;
} catch (InvalidPhoneException ex) {
return UserOperationState.PHONE_EXIST;
} catch (ValidationException ex) {
Logger.getLogger(UserService.class.getName()).log(Level.SEVERE, null, ex);
}
}
try {
userDao.create(user);
return UserOperationState.CREATE_SUCCESS;
} catch (EJBException e) {
return UserOperationState.CREATE_ERROR;
}
}
i tutaj właśnie to co mi się nie podoba to te 4 catche, w dodatku muszę obsługiwać wyjątek ValidationException bo mam listę typu interfejsu. Pomyślałem, że lepszym rozwiązaniem niż te 4 catche będzie pominięcie wyjątków. Ogólnie czy te wyjątki to dobry pomysł? Przecież wprowadzenie przez użytkownika błędnego adresu email, telefonu czy pesel nie jest żadną sytuacją wyjątkową, tylko normalnym błędem na porządku dziennym. Nie mylę się, że te wyjątki byłyby trochę na wyrost? Postanowiłem to trochę zmodyfikować wyrzucając w ogóle wyjątki a klasy walidatorów w taki sposób, że zamiast rzucać wyjątek będą zwracać odpowiedni UserOperationState. Kod poniżej:
Przykładowa zmodyfikowana klasa walidatora, nie rzuca już wyjątków, tylko w wypadku gdy napotka na błąd zwraca odpowiedni stan w przeciwnym wypadku zwraca nulla(tu mam wątpliwości czy takie zwracania nulla jest ok)
Kopiuj
public class EmailValidator implements UserDataValidator{
@PersistenceContext
private EntityManager EntityManager;
@Override
public UserOperationState validateCreateUser(User user){
if ((user.getEmail() != null) && (findEmailExist(user.getEmail()))) {
return UserOperationState.EMAIL_EXIST;
}
return null;
}
@Override
public UserOperationState validateUpdateUser(User user){
if ((user.getEmail() != null) && (findEmailExist(user.getEmail(), user.getId()))) {
return UserOperationState.EMAIL_EXIST;
}
return null;
}
Wtedy klasa w serwisie wygląda tak:
Kopiuj
@Override
public UserOperationState create(User user) {
for(UserDataValidator validator : userDataValidatorList ) {
UserOperationState userOperationState = validator.validateCreateUser(user);
if (userOperationState != null) {
return userOperationState;
}
}
try {
userDao.create(user);
return UserOperationState.CREATE_SUCCESS;
} catch (EJBException e) {
return UserOperationState.CREATE_ERROR;
}
}
Wydaje mi się być zwięźlejsza. Lecimy po kolekcji walidatorów i jeśli, któryś z nich zwróci coś innego niż null to kończymy metodę poprzez zwrócenie odpowiedniego UserOperationState.
