JAVA - Validating User inputs on service methods in a cleaner way

Question

• In a project, we use service methods, there, we have to validate certain required inputs for our business logic to work:

public void exampleMethod(final UserDto user, String sthElse) {
 if (user.getId() == null) {
 throw new CustomHandledException("example");
 }
 if (sthElse == null) {
 throw new CustomHandledException("example");
 }
 GetUserResponseDto getUserResponse = client.getUserById(user.getId(), country);
 if (getUserResponse == null) {
 if (user.getGivenName() == null) {
 throw new CustomHandledException("example");
 }
 if (user.getEmail() == null) {
 throw new CustomHandledException("example");
 }
 CreateUserRequestDto createUserBody = UserMapper.toCreateBody(user);
 client.createUser(createUserBody);
 return;
 }
 if (user.getLocale().equals(getUserResponse.getUser().getProfile().getLocale())){
 return;
 }
 client.updateUser(user);
 }

Even though the code isn't necessarily wrong and is quite understandable (for me) I feel like there is something odd there, do you have any suggestions for a cleaner code?

I tried to write a clean code, however, doesn't feel like it is clean.

Answer 1

I'm not sure what you're getting at about it seeming odd. I suppose the various if/throws are repetitive and could be reduced by writing a function for them.

private void requireNotNull(boolean requirementMet, String errorMessage) {
 if (requirementMet == null) throw CustomHandledException(errorMessage);
}
// or, depending on whether the messages require any calculations:
private void requireNotNull(boolean requirementMet, Supplier<String> lazyErrorMessage) {
 if (requirementMet == null) throw CustomHandledException(lazyErrorMessage.get());
}

This is getting really nitpicky, but I think the logic flow would read slightly easier if you handle the non-null case of getUserResponse first so you don't have a big chunk of code in between retrieving something and working with that retrieved thing.

public void exampleMethod(final UserDto user, String sthElse) {
 requireNotNull(user.getId(), "example");
 requireNotNull(sthElse, "example");
 GetUserResponseDto getUserResponse = client.getUserById(user.getId(), country);
 if (getUserResponse != null) { 
 Locale currentLocale = getUserResponse.getUser().getProfile().getLocale();
 if (!user.getLocale().equals(currentLocale)) {
 client.updateUser(user);
 }
 } else {
 requireNotNull(user.getGivenName(), "example");
 requireNotNull(user.getEmail(), "example");
 CreateUserRequestDto createUserBody = UserMapper.toCreateBody(user);
 client.createUser(createUserBody);
 }
}

Aside from that, I wouldn't start variable names with verbs.

• RoToRa: No need to write requireNontNull yourself. Java comes with that: docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/…
• Tenfour04: @RoToRa, depends on whether they specifically need that "CustomHandled" exception.

Answer 2

Separate into any function

private void validateInputs(final UserDto user, String sthElse) {
 if (user.getId() == null || sthElse == null) {
 throw new CustomHandledException("example");
 }
 
 if (user.getGivenName() == null || user.getEmail() == null) {
 throw new CustomHandledException("example");
 }
}
private void handleUserCreation(final UserDto user) {
 CreateUserRequestDto createUserBody = UserMapper.toCreateBody(user);
 client.createUser(createUserBody);
}
private void handleUserUpdate(final UserDto user, GetUserResponseDto getUserResponse) {
 if (!user.getLocale().equals(getUserResponse.getUser().getProfile().getLocale())) {
 client.updateUser(user);
 }
}

And then combine it in:

public void exampleMethod(final UserDto user, String sthElse) {
 validateInputs(user, sthElse);
 GetUserResponseDto getUserResponse = client.getUserById(user.getId(), country);
 if (getUserResponse == null) {
 handleUserCreation(user);
 } else {
 handleUserUpdate(user, getUserResponse);
 }
}

• Kaue: thanks for the support, definitely gave me some insights
• Tenfour04: I suggest avoiding weak words like "handle" in method names. It doesn't add any meaning to the name, so it becomes a hindrance to readability. These could be "createUser" and "updateUser".