1

I have lots of method and on each method I have to do a validation. Currently, my code look like this and it works fine.

@Service
public class UserService {
    @Autowired
    private UserValidation userValidation;

    public BaseResponse<AuthenticationResponse> login(UserLoginRequest request) {
        List<ErrorCode> errors = userValidation.validateUserLoginRequest(request);
        if (!errors.isEmpty()) return Utils.constructFailedBaseResponse(errors);
        // ...
    }

    public BaseResponse<AuthenticationResponse> register(UserRegisterRequest request) {
        List<ErrorCode> errors = userValidation.validateUserRegisterRequest(request);
        if (!errors.isEmpty()) return Utils.constructFailedBaseResponse(errors);
        // ...
    }

    public BaseResponse<User> view(String username) {
        List<ErrorCode> errors = userValidation.validateUserUsernameExists(username);
        if (!errors.isEmpty()) return Utils.constructFailedBaseResponse(errors);
        // ...
    }

    public BaseResponse<Void> edit(UserEditRequest request) {
        List<ErrorCode> errors = userValidation.validateUserEditRequest(request);
        if (!errors.isEmpty()) return Utils.constructFailedBaseResponse(errors);
        // ...
    }
}

However, I'm curios is there a way to refactor the code so that I dont need to do the repetitive if statements on every method. The code below is what I'm trying to achiev, but I don't know how. Any idea how to convert the code above to the code below? Another solution to refactor is very welcome.

(Or should I just stick with my code above? why?)

@Service
public class UserService {
    @Autowired
    private UserValidation userValidation;

    public BaseResponse<AuthenticationResponse> login(UserLoginRequest request) {
        userValidation.validateUserLoginRequest(request);
        // ...
    }

    public BaseResponse<AuthenticationResponse> register(UserRegisterRequest request) {
        userValidation.validateUserRegisterRequest(request);
        // ...
    }

    public BaseResponse<User> view(String username) {
        userValidation.validateUserUsernameExists(username);
        // ...
    }

    public BaseResponse<Void> edit(UserEditRequest request) {
        userValidation.validateUserEditRequest(request);
        // ...
    }
}
Justin Xu
  • 13
  • 3
  • Since you do the same in the if condition part, you can extract that into a separate method. That way this will look more cleaner. – chameerar Jun 27 '23 at 03:16
  • @chameerar I'm not sure about the `return` part. I guess it is not possible to extract the if into a method. – Justin Xu Jun 27 '23 at 03:51
  • 1
    What you want to achieve is called [*aspect programming*](https://en.m.wikipedia.org/wiki/Aspect-oriented_programming). I would annotate the methods accordingly and let the code behind the annotation handle it. Either something like AspectJ where you write the code, or Spring Security where it's done mostly magically. – Bohemian Jun 27 '23 at 04:19

0 Answers0